netdev - Re: [PATCH net] atlantic: fix deadlock at aq_nic

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: <20221019083913.09437041@kernel.org>
Date:   Wed, 19 Oct 2022 08:39:13 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Íñigo Huguet <ihuguet@...hat.com>
Cc:     Andrew Lunn <andrew@...n.ch>, irusskikh@...vell.com,
        dbogdanov@...vell.com, davem@...emloft.net, edumazet@...gle.com,
        pabeni@...hat.com, netdev@...r.kernel.org,
        Li Liang <liali@...hat.com>
Subject: Re: [PATCH net] atlantic: fix deadlock at aq_nic_stop

On Wed, 19 Oct 2022 08:18:29 +0200 Íñigo Huguet wrote:
> Yes, now I get it.
> 
> However, I think I won't use this strategy this time: rtnl_lock is
> only needed in the work task if IS_ENABLED(CONFIG_MACSEC). Acquiring
> rtnl_lock every time if macsec is not enabled wouldn't be protecting
> anything, so it would be a waste. I think that the strategy suggested
> by Andrew of adding a dedicated mutex to protect atlantic's macsec
> operations makes more sense in this case. Do you agree?

Dunno, locks don't protect operations, they protect state (as the link
Andrew sent probably explains?), so it's hard to say how easily you can
inject a new lock into this driver covering relevant bits. My gut
feeling is that refcounting would be less error prone. But I don't feel
strongly enough to force one choice over the other.