linux-kernel - Re: [PATCH net-next] devlink: Require devlink lock during device reload

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: <20211108101646.0a4e5ca4@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Mon, 8 Nov 2021 10:16:46 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Leon Romanovsky <leon@...nel.org>
Cc:     Ido Schimmel <idosch@...sch.org>, Jiri Pirko <jiri@...nulli.us>,
        "David S . Miller" <davem@...emloft.net>,
        Jiri Pirko <jiri@...dia.com>, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org, edwin.peer@...adcom.com
Subject: Re: [PATCH net-next] devlink: Require devlink lock during device
 reload

On Mon, 8 Nov 2021 19:32:19 +0200 Leon Romanovsky wrote:
> > I think it's common sense. We're just exporting something to make our
> > lives easier somewhere else in the three. Do you see a way in which
> > taking refs on devlink can help out-of-tree code?  
> 
> I didn't go such far in my thoughts. My main concern is that you ore
> exposing broken devlink internals in the hope that drivers will do better
> locking. I wanted to show that internal locking should be fixed first.
> 
> https://lore.kernel.org/netdev/cover.1636390483.git.leonro@nvidia.com/T/#m093f067d0cafcbe6c05ed469bcfd708dd1eb7f36
> 
> While this series fixes locking and after all my changes devlink started
> to be more secure, that works correctly for simple drivers.

I prefer my version. I think I asked you to show how the changes make
drivers simpler, which you failed to do.

I already told you how this is going to go, don't expect me to comment
too much.

> However for net namespace aware drivers it still stays DOA.
> 
> As you can see, devlink reload holds pernet_ops_rwsem, which drivers should
> take too in order to unregister_netdevice_notifier.
> 
> So for me, the difference between netdevsim and real device (mlx5) is
> too huge to really invest time into netdevsim-centric API, because it
> won't solve any of real world problems.

Did we not already go over this? Sorry, it feels like you're repeating
arguments which I replied to before. This is exhausting.

nfp will benefit from the simplified locking as well, and so will bnxt,
although I'm not sure the maintainers will opt for using devlink framework
due to the downstream requirements.