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

Message-ID: <fbf9dae9-c023-4b15-b3d8-6b19240f59b0@linux.intel.com>
Date: Fri, 29 Mar 2024 12:31:58 +0100
From: Marcin Szycik <marcin.szycik@...ux.intel.com>
To: Simon Horman <horms@...nel.org>
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 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