netdev - Re: [Intel-wired-lan] [PATCH iwl-next v3] ice: Reset VF on Tx MDD event

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: <20240331182747.GC26556@kernel.org>
Date: Sun, 31 Mar 2024 19:27:47 +0100
From: Simon Horman <horms@...nel.org>
To: Marcin Szycik <marcin.szycik@...ux.intel.com>
Cc: Wojciech Drewek <wojciech.drewek@...el.com>, netdev@...r.kernel.org,
	pawel.chmielewski@...el.com, anthony.l.nguyen@...el.com,
	Liang-Min Wang <liang-min.wang@...el.com>,
	intel-wired-lan@...ts.osuosl.org
Subject: Re: [Intel-wired-lan] [PATCH iwl-next v3] ice: Reset VF on Tx MDD
 event

On Fri, Mar 29, 2024 at 12:31:58PM +0100, Marcin Szycik wrote:
> 
> 
> On 28.03.2024 18:34, Simon Horman wrote:
> > On Tue, Mar 26, 2024 at 05:44:55PM +0100, Marcin Szycik wrote:
> >> In cases when VF sends malformed packets that are classified as malicious,
> >> sometimes it causes Tx queue to freeze. This frozen queue can be stuck
> >> for several minutes being unusable. This behavior can be reproduced with
> >> a faulty userspace app running on VF.
> >>
> >> When Malicious Driver Detection event occurs and the mdd-auto-reset-vf
> >> private flag is set, perform a graceful VF reset to quickly bring VF back
> >> to operational state. Add a log message to notify about the cause of
> >> the reset. Add a helper for this to be reused for both TX and RX events.
> >>
> >> Reviewed-by: Wojciech Drewek <wojciech.drewek@...el.com>
> >> Co-developed-by: Liang-Min Wang <liang-min.wang@...el.com>
> >> Signed-off-by: Liang-Min Wang <liang-min.wang@...el.com>
> >> Signed-off-by: Marcin Szycik <marcin.szycik@...ux.intel.com>
> > 
> > Hi Marcin,
> > 
> > If I read this correctly then a reset may be performed for several
> > different conditions - values of different registers - for a VF
> > as checked in a for loop.
> > 
> > I am wondering if multiple resets could occur for the same VF within
> > an iteration of the for loop - because more than one of the conditions is
> > met. And, if so, is this ok?
> 
> Hi Simon,
> 
> Good point. Nothing too bad should happen, as ice_reset_vf() acquires mutex lock
> (in fact two locks), so several resets would just happen in sequence. However,
> it doesn't make much sense to reset VF multiple times, so maybe instead of issuing
> reset on each condition, I'll set some flag, and after checking all registers I'll
> trigger reset if that flag is set. What do you think?

Thanks Marcin,

FWIIW, that sounds like a good approach to me.

-- 
pw-bot: changes-requested