netdev - Re: [PATCH net-next v2 4/4] ethtool: don't drop the rtnl

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YX44VqMfYDaDFi5r@unreal>
Date:   Sun, 31 Oct 2021 08:31:50 +0200
From:   Leon Romanovsky <leon@...nel.org>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, jiri@...nulli.us,
        mkubecek@...e.cz, andrew@...n.ch, f.fainelli@...il.com
Subject: Re: [PATCH net-next v2 4/4] ethtool: don't drop the rtnl_lock half
 way thru the ioctl

On Sat, Oct 30, 2021 at 10:18:51AM -0700, Jakub Kicinski wrote:
> devlink compat code needs to drop rtnl_lock to take
> devlink->lock to ensure correct lock ordering.
> 
> This is problematic because we're not strictly guaranteed
> that the netdev will not disappear after we re-lock.
> It may open a possibility of nested ->begin / ->complete
> calls.
> 
> Instead of calling into devlink under rtnl_lock take
> a ref on the devlink instance and make the call after
> we've dropped rtnl_lock.
> 
> We (continue to) assume that netdevs have an implicit
> reference on the devlink returned from ndo_get_devlink_port
> 
> Note that ndo_get_devlink_port will now get called
> under rtnl_lock. That should be fine since none of
> the drivers seem to be taking serious locks inside
> ndo_get_devlink_port.
> 
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
>  include/net/devlink.h |  8 ++++----
>  net/core/devlink.c    | 45 +++++++------------------------------------
>  net/ethtool/ioctl.c   | 36 ++++++++++++++++++++++++++++++----
>  3 files changed, 43 insertions(+), 46 deletions(-)
> 

Thanks,
Reviewed-by: Leon Romanovsky <leonro@...dia.com>