linux-kernel - Re: [PATCH net 0/2] Fix VF to VM attach detach

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]

Date:   Tue, 8 Aug 2023 15:10:59 -0700
From:   Jacob Keller <jacob.e.keller@...el.com>
To:     Petr Oros <poros@...hat.com>, <netdev@...r.kernel.org>
CC:     <jesse.brandeburg@...el.com>, <anthony.l.nguyen@...el.com>,
        <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
        <pabeni@...hat.com>, <przemyslawx.patynowski@...el.com>,
        <kamil.maziarz@...el.com>, <dawidx.wesierski@...el.com>,
        <mateusz.palczewski@...el.com>, <slawomirx.laba@...el.com>,
        <norbertx.zulinski@...el.com>, <intel-wired-lan@...ts.osuosl.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net 0/2] Fix VF to VM attach detach

On 8/7/2023 2:48 AM, Petr Oros wrote:
> Petr Oros (2):
>   Revert "ice: Fix ice VF reset during iavf initialization"
>   ice: Fix NULL pointer deref during VF reset
> 
>  drivers/net/ethernet/intel/ice/ice_sriov.c    |  8 ++---
>  drivers/net/ethernet/intel/ice/ice_vf_lib.c   | 34 +++++--------------
>  drivers/net/ethernet/intel/ice/ice_vf_lib.h   |  1 -
>  drivers/net/ethernet/intel/ice/ice_virtchnl.c |  1 -
>  4 files changed, 12 insertions(+), 32 deletions(-)
> 

I reviewed the commit message for the reverted commit and I concur that
any sort of issue that it fixed with respect to concurrent resets is
better fixed by the 2nd commit of this series.

The motivation for the original commit appears to be to prevent a reset
from happening while the VF is still resetting. I think this motivation
is incorrect for a few reasons:

1) as described by the revert above, if the VF has not had iAVF load on
it yet (such as when its been assigned to the pass through PCI driver),
it will never initialize, and thus we can't ever reset the device.

2) It does not make sense for the PF to rely on or assume behavior of
the VF (i.e. that it will properly initialize) and I believe it should
be allowed to reset the device without regard to the state of the VF
driver. It is a bug in the VF driver if this causes problems for it. I
think we recently fixed several issues here in iAVF.

With that in mind, I think this series of fixes is good.

Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>