Re: [PATCH net-next 1/3] net: bridge: fix ioctl locking

[Nikolay Aleksandrov <nikolay@nvidia.com>]

On 05/08/2021 11:29, Nikolay Aleksandrov wrote:
> From: Nikolay Aleksandrov <nikolay@nvidia.com>
> 
> Before commit ad2f99aedf8f ("net: bridge: move bridge ioctls out of
> .ndo_do_ioctl") the bridge ioctl calls were divided in two parts:
> one was deviceless called by sock_ioctl and didn't expect rtnl to be held,
> the other was with a device called by dev_ifsioc() and expected rtnl to be
> held. After the commit above they were united in a single ioctl stub, but
> it didn't take care of the locking expectations.
> For sock_ioctl now we acquire  (1) br_ioctl_mutex, (2) rtnl
> and for dev_ifsioc we acquire  (1) rtnl,           (2) br_ioctl_mutex
> 
> The fix is to get a refcnt on the netdev for dev_ifsioc calls and drop rtnl
> then to reacquire it in the bridge ioctl stub after br_ioctl_mutex has
> been acquired. That will avoid playing locking games and make the rules
> straight-forward: we always take br_ioctl_mutex first, and then rtnl.
> 
> Reported-by: syzbot+34fe5894623c4ab1b379@syzkaller.appspotmail.com
> Fixes: ad2f99aedf8f ("net: bridge: move bridge ioctls out of .ndo_do_ioctl")
> Signed-off-by: Nikolay Aleksandrov <nikolay@nvidia.com>
> ---
>  net/bridge/br_if.c    |  4 +---
>  net/bridge/br_ioctl.c | 37 ++++++++++++++++++++++++-------------
>  net/core/dev_ioctl.c  |  7 ++++++-
>  3 files changed, 31 insertions(+), 17 deletions(-)
> 
[snip]

I fixed the bridge side of things, but the unlock/lock suggestion was made first by Hillf.
I forgot to add:

Suggested-by: Hillf Danton <hdanton@sina.com>

+CC Hillf

Hillf, since the rtnl unlock/lock suggestion was yours feel free to add
your signed-off-by

Thanks,
 Nik