| Message ID | 20210914030150.5838-1-yajun.deng@linux.dev (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: core: fix the order in dev_put() and rtnl_lock() | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 3 maintainers not CCed: arnd@arndb.de cong.wang@bytedance.com colin.king@canonical.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 4 this patch: 4 |
| netdev/kdoc | success | Errors and warnings before: 1 this patch: 1 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 9 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 4 this patch: 4 |
| netdev/header_inline | success | Link |
On 9/13/21 8:01 PM, Yajun Deng wrote: > The dev_put() should be after rtnl_lock() in case for race. > > Fixes: 893b19587534 ("net: bridge: fix ioctl locking") > Signed-off-by: Yajun Deng <yajun.deng@linux.dev> > --- > net/core/dev_ioctl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c > index 0e87237fd871..9796fa35fe88 100644 > --- a/net/core/dev_ioctl.c > +++ b/net/core/dev_ioctl.c > @@ -384,8 +384,8 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, void __user *data, > dev_hold(dev); > rtnl_unlock(); > err = br_ioctl_call(net, netdev_priv(dev), cmd, ifr, NULL); > - dev_put(dev); > rtnl_lock(); > + dev_put(dev); > return err; > > case SIOCSHWTSTAMP: > What race exactly are you trying to avoid ? This patch does not look needed to me.
On 14/09/2021 08:18, Eric Dumazet wrote: > > > On 9/13/21 8:01 PM, Yajun Deng wrote: >> The dev_put() should be after rtnl_lock() in case for race. >> >> Fixes: 893b19587534 ("net: bridge: fix ioctl locking") >> Signed-off-by: Yajun Deng <yajun.deng@linux.dev> >> --- >> net/core/dev_ioctl.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c >> index 0e87237fd871..9796fa35fe88 100644 >> --- a/net/core/dev_ioctl.c >> +++ b/net/core/dev_ioctl.c >> @@ -384,8 +384,8 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, void __user *data, >> dev_hold(dev); >> rtnl_unlock(); >> err = br_ioctl_call(net, netdev_priv(dev), cmd, ifr, NULL); >> - dev_put(dev); >> rtnl_lock(); >> + dev_put(dev); >> return err; >> >> case SIOCSHWTSTAMP: >> > > What race exactly are you trying to avoid ? > > This patch does not look needed to me. > +1 There isn't a race there
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c index 0e87237fd871..9796fa35fe88 100644 --- a/net/core/dev_ioctl.c +++ b/net/core/dev_ioctl.c @@ -384,8 +384,8 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, void __user *data, dev_hold(dev); rtnl_unlock(); err = br_ioctl_call(net, netdev_priv(dev), cmd, ifr, NULL); - dev_put(dev); rtnl_lock(); + dev_put(dev); return err; case SIOCSHWTSTAMP:
The dev_put() should be after rtnl_lock() in case for race. Fixes: 893b19587534 ("net: bridge: fix ioctl locking") Signed-off-by: Yajun Deng <yajun.deng@linux.dev> --- net/core/dev_ioctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)