netdev - Re: [PATCH net 2/2] ice: Fix call trace with null VSI during VF reset

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] [day] [month] [year] [list]

Message-ID: <20220817084748.7ed9ee99@kernel.org>
Date:   Wed, 17 Aug 2022 08:47:48 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     "Sokolowski, Jan" <jan.sokolowski@...el.com>
Cc:     "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "Jaron, MichalX" <michalx.jaron@...el.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "Jagielski, Jedrzej" <jedrzej.jagielski@...el.com>,
        "Szlosek, Marek" <marek.szlosek@...el.com>
Subject: Re: [PATCH net 2/2] ice: Fix call trace with null VSI during VF
 reset

On Wed, 17 Aug 2022 13:31:00 +0000 Sokolowski, Jan wrote:
> I'd like to send this response in Michal Jaron's name, as he currently cannot respond to this email.
> 
> Generally you are right, it is better idea to try to keep the system
> in a consistent state than adding "if NULL return;" but I don't think
> it will work here. This "if NULL return;" is here because of race
> between two resets and I don't think we can guarantee that this race
> will be not present if we flush the service work before reset. The
> problem is that vf reset is called in the same time from vf on vm and
> from pf. When reset from vf is called and reset form pf don't clear
> rings yet we must go into ice_reset_vf and clear those rings without
> triggering second reset. If we don't clear rings there we may trigger
> page_frag_cache_drain crash related to writing data to unmapped
> queues. In such cases if there are no vsis we don't need to do this
> and this WARN_ON is not necessary, but we need to check it anyway.

Hm, I assumed there's some synchronization here which we can use 
to prevent the race. After all _something_ must ensure the pointer
returned from ice_get_vf_vsi() will not go away, for instance.

But I'm obviously speculating and Dave already applied the patches 
so moving on.. :)