We are still chasing some syzbot reports where we think a rogue dev_put() is called with no corresponding prior dev_hold(). Unfortunately it eats a reference on dev->dev_refcnt taken by innocent dev_hold_track(), meaning that the refcount saturation splat comes too late to be useful. Make sure that 'not tracked' dev_put() and dev_hold() better use CONFIG_NET_DEV_REFCNT_TRACKER=y debug infrastructure: Prior patch in the series allowed ref_tracker_alloc() and ref_tracker_free() to be called with a NULL @trackerp parameter, and to use a separate refcount only to detect too many put() even in the following case: dev_hold_track(dev, tracker_1, GFP_ATOMIC); dev_hold(dev); dev_put(dev); dev_put(dev); // Should complain loudly here. dev_put_track(dev, tracker_1); // instead of here Add clarification about netdev_tracker_alloc() role. v2: I replaced the dev_put() in linkwatch_do_dev() with __dev_put() because callers called netdev_tracker_free().
Showing the changes to include (1 of 3 files). Show all
Unified · Split
| @@ -3817,14 +3817,7 @@ extern unsigned int netdev_budget_usecs; · 3816 unmodified lines | ||
| 3817 | 3817 | /* Called by rtnetlink.c:rtnl_unlock() */ |
| 3818 | 3818 | void netdev_run_todo(void); |
| 3819 | 3819 | |
| 3820 | −/** | |
| 3821 | − * dev_put - release reference to device | |
| 3822 | − * @dev: network device | |
| 3823 | − * | |
| 3824 | − * Release reference to device to allow it to be freed. | |
| 3825 | − * Try using dev_put_track() instead. | |
| 3826 | − */ | |
| 3827 | −static inline void dev_put(struct net_device *dev) | |
| 3820 | +static inline void __dev_put(struct net_device *dev) | |
| 3828 | 3821 | { |
| 3829 | 3822 | if (dev) { |
| 3830 | 3823 | #ifdef CONFIG_PCPU_DEV_REFCNT |
| @@ -3835,14 +3828,7 @@ static inline void dev_put(struct net_device *dev) · 4 unmodified lines | ||
| 3835 | 3828 | } |
| 3836 | 3829 | } |
| 3837 | 3830 | |
| 3838 | −/** | |
| 3839 | − * dev_hold - get reference to device | |
| 3840 | − * @dev: network device | |
| 3841 | − * | |
| 3842 | − * Hold reference to device to keep it from being freed. | |
| 3843 | − * Try using dev_hold_track() instead. | |
| 3844 | − */ | |
| 3845 | −static inline void dev_hold(struct net_device *dev) | |
| 3831 | +static inline void __dev_hold(struct net_device *dev) | |
| 3846 | 3832 | { |
| 3847 | 3833 | if (dev) { |
| 3848 | 3834 | #ifdef CONFIG_PCPU_DEV_REFCNT |
| @@ -3853,11 +3839,24 @@ static inline void dev_hold(struct net_device *dev) · 4 unmodified lines | ||
| 3853 | 3839 | } |
| 3854 | 3840 | } |
| 3855 | 3841 | |
| 3842 | +static inline void __netdev_tracker_alloc(struct net_device *dev, | |
| 3843 | + netdevice_tracker *tracker, | |
| 3844 | + gfp_t gfp) | |
| 3845 | +{ | |
| 3846 | +#ifdef CONFIG_NET_DEV_REFCNT_TRACKER | |
| 3847 | + ref_tracker_alloc(&dev->refcnt_tracker, tracker, gfp); | |
| 3848 | +#endif | |
| 3849 | +} | |
| 3850 | + | |
| 3851 | +/* netdev_tracker_alloc() can upgrade a prior untracked reference | |
| 3852 | + * taken by dev_get_by_name()/dev_get_by_index() to a tracked one. | |
| 3853 | + */ | |
| 3856 | 3854 | static inline void netdev_tracker_alloc(struct net_device *dev, |
| 3857 | 3855 | netdevice_tracker *tracker, gfp_t gfp) |
| 3858 | 3856 | { |
| 3859 | 3857 | #ifdef CONFIG_NET_DEV_REFCNT_TRACKER |
| 3860 | − ref_tracker_alloc(&dev->refcnt_tracker, tracker, gfp); | |
| 3858 | + refcount_dec(&dev->refcnt_tracker.no_tracker); | |
| 3859 | + __netdev_tracker_alloc(dev, tracker, gfp); | |
| 3861 | 3860 | #endif |
| 3862 | 3861 | } |
| 3863 | 3862 | |
| @@ -3873,8 +3872,8 @@ static inline void dev_hold_track(struct net_device *dev, · 9 unmodified lines | ||
| 3873 | 3872 | netdevice_tracker *tracker, gfp_t gfp) |
| 3874 | 3873 | { |
| 3875 | 3874 | if (dev) { |
| 3876 | − dev_hold(dev); | |
| 3877 | − netdev_tracker_alloc(dev, tracker, gfp); | |
| 3875 | + __dev_hold(dev); | |
| 3876 | + __netdev_tracker_alloc(dev, tracker, gfp); | |
| 3878 | 3877 | } |
| 3879 | 3878 | } |
| 3880 | 3879 | |
| @@ -3883,10 +3882,34 @@ static inline void dev_put_track(struct net_device *dev, · 2 unmodified lines | ||
| 3883 | 3882 | { |
| 3884 | 3883 | if (dev) { |
| 3885 | 3884 | netdev_tracker_free(dev, tracker); |
| 3886 | − dev_put(dev); | |
| 3885 | + __dev_put(dev); | |
| 3887 | 3886 | } |
| 3888 | 3887 | } |
| 3889 | 3888 | |
| 3889 | +/** | |
| 3890 | + * dev_hold - get reference to device | |
| 3891 | + * @dev: network device | |
| 3892 | + * | |
| 3893 | + * Hold reference to device to keep it from being freed. | |
| 3894 | + * Try using dev_hold_track() instead. | |
| 3895 | + */ | |
| 3896 | +static inline void dev_hold(struct net_device *dev) | |
| 3897 | +{ | |
| 3898 | + dev_hold_track(dev, NULL, GFP_ATOMIC); | |
| 3899 | +} | |
| 3900 | + | |
| 3901 | +/** | |
| 3902 | + * dev_put - release reference to device | |
| 3903 | + * @dev: network device | |
| 3904 | + * | |
| 3905 | + * Release reference to device to allow it to be freed. | |
| 3906 | + * Try using dev_put_track() instead. | |
| 3907 | + */ | |
| 3908 | +static inline void dev_put(struct net_device *dev) | |
| 3909 | +{ | |
| 3910 | + dev_put_track(dev, NULL); | |
| 3911 | +} | |
| 3912 | + | |
| 3890 | 3913 | static inline void dev_replace_track(struct net_device *odev, |
| 3891 | 3914 | struct net_device *ndev, |
| 3892 | 3915 | netdevice_tracker *tracker, |
| @@ -3895,11 +3918,11 @@ static inline void dev_replace_track(struct net_device *odev, · 2 unmodified lines | ||
| 3895 | 3918 | if (odev) |
| 3896 | 3919 | netdev_tracker_free(odev, tracker); |
| 3897 | 3920 | |
| 3898 | − dev_hold(ndev); | |
| 3899 | − dev_put(odev); | |
| 3921 | + __dev_hold(ndev); | |
| 3922 | + __dev_put(odev); | |
| 3900 | 3923 | |
| 3901 | 3924 | if (ndev) |
| 3902 | − netdev_tracker_alloc(ndev, tracker, gfp); | |
| 3925 | + __netdev_tracker_alloc(ndev, tracker, gfp); | |
| 3903 | 3926 | } |
| 3904 | 3927 | |
| 3905 | 3928 | /* Carrier loss detection, dial on demand. The functions netif_carrier_on |