netdev - Re: [PATCH net-next 5/5] devlink: hold the instance lock during eswitch

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 for Android: free password hash cracker in your pocket

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

Message-ID: <YjLoqpwsgJdXeIGq@unreal>
Date:   Thu, 17 Mar 2022 09:52:10 +0200
From:   Leon Romanovsky <leon@...nel.org>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, jiri@...nulli.us,
        saeedm@...dia.com, idosch@...sch.org, michael.chan@...adcom.com,
        simon.horman@...igine.com
Subject: Re: [PATCH net-next 5/5] devlink: hold the instance lock during
 eswitch_mode callbacks

On Wed, Mar 16, 2022 at 09:20:23PM -0700, Jakub Kicinski wrote:
> Make the devlink core hold the instance lock during eswitch_mode
> callbacks. Cheat in case of mlx5 (see the cover letter).

And this is one the main difference between your and mine proposals/solutions.
I didn't want to cheat as it doesn't help to the end goal - remove devlink_mutex.

Can you please change the comments in mlx5 to be more direct?
"/* TODO: convert the driver to devl_* */"
->
"/* FIXME: devl_unlock() followed by devl_lock() inside driver callback
  * never correct and prone to races. Never repeat this pattern.
  *
  * This code MUST be fixed before removing devlink_mutex as it is safe
  * to do only because of that mutex.
  */"

Something like that.

The code is correct, but like I said before, I don't like the direction.
I expect that this anti-pattern is going to be copy/pasted very soon.

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