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: <20221018085906.76f70073@kernel.org>
Date:   Tue, 18 Oct 2022 08:59:06 -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 Tue, 18 Oct 2022 08:15:38 +0200 Íñigo Huguet wrote:
> Interesting solution, I didn't even think of something like this.
> However, despite not being 100% sure, I think that it's not valid in
> this case because the work's task communicates with fw and uses
> resources that are deinitialized at ndo_stop. That's why I think that
> just holding a reference to the device is not enough.

You hold a reference to the netdev just to be able to take rtnl_lock()
and check if it's still running. If it is UP you're protected from it
going down due to rtnl_lock you took. If it's DOWN, as you say, it's not
safe to access all the bits so just unlock and return.

But because you're holding the reference it's safe to cancel_work()
without _sync on down, because the work itself will check if it should
have been canceled.

Dunno if that's a good explanation :S