2021-09-14 03:04:11

by Yajun Deng

Subject: [PATCH net-next] net: core: fix the order in dev_put() and rtnl_lock()

The dev_put() should be after rtnl_lock() in case for race.

Fixes: 893b19587534 ("net: bridge: fix ioctl locking")
Signed-off-by: Yajun Deng <[email protected]>
---
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:
--
2.32.0

2021-09-14 05:19:23

by Eric Dumazet

Subject: Re: [PATCH net-next] net: core: fix the order in dev_put() and rtnl_lock()

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 <[email protected]>
> ---
> 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.

2021-09-14 07:53:54

by Nikolay Aleksandrov

Subject: Re: [PATCH net-next] net: core: fix the order in dev_put() and rtnl_lock()

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 <[email protected]>
>> ---
>> 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