Message-ID: <c58fe076-3425-394f-b7da-c6df6ac45d98@intel.com>
Date:   Tue, 14 Mar 2023 14:26:10 -0700
From:   Jacob Keller <jacob.e.keller@...el.com>
To:     Leon Romanovsky <leon@...nel.org>,
        Tony Nguyen <anthony.l.nguyen@...el.com>
CC:     <davem@...emloft.net>, <kuba@...nel.org>, <pabeni@...hat.com>,
        <edumazet@...gle.com>, <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 00/14][pull request] ice: refactor mailbox
 overflow detection



On 3/14/2023 6:57 AM, Leon Romanovsky wrote:
> On Mon, Mar 13, 2023 at 11:21:09AM -0700, Tony Nguyen wrote:
>> Jake Keller says:
>>
>> The primary motivation of this series is to cleanup and refactor the mailbox
>> overflow detection logic such that it will work with Scalable IOV. In
>> addition a few other minor cleanups are done while I was working on the
>> code in the area.
>>
>> First, the mailbox overflow functions in ice_vf_mbx.c are refactored to
>> store the data per-VF as an embedded structure in struct ice_vf, rather than
>> stored separately as a fixed-size array which only works with Single Root
>> IOV. This reduces the overall memory footprint when only a handful of VFs
>> are used.
>>
>> The overflow detection functions are also cleaned up to reduce the need for
>> multiple separate calls to determine when to report a VF as potentially
>> malicious.
>>
>> Finally, the ice_is_malicious_vf function is cleaned up and moved into
>> ice_virtchnl.c since it is not Single Root IOV specific, and thus does not
>> belong in ice_sriov.c
>>
>> I could probably have done this in fewer patches, but I split pieces out to
>> hopefully aid in reviewing the overall sequence of changes. This does cause
>> some additional thrash as it results in intermediate versions of the
>> refactor, but I think its worth it for making each step easier to
>> understand.
>>
>> The following are changes since commit 95b744508d4d5135ae2a096ff3f0ee882bcc52b3:
>>   qede: remove linux/version.h and linux/compiler.h
>> and are available in the git repository at:
>>   git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue 100GbE
>>
>> Jacob Keller (14):
>>   ice: re-order ice_mbx_reset_snapshot function
>>   ice: convert ice_mbx_clear_malvf to void and use WARN
>>   ice: track malicious VFs in new ice_mbx_vf_info structure
>>   ice: move VF overflow message count into struct ice_mbx_vf_info
>>   ice: remove ice_mbx_deinit_snapshot
>>   ice: merge ice_mbx_report_malvf with ice_mbx_vf_state_handler
>>   ice: initialize mailbox snapshot earlier in PF init
>>   ice: declare ice_vc_process_vf_msg in ice_virtchnl.h
>>   ice: always report VF overflowing mailbox even without PF VSI
>>   ice: remove unnecessary &array[0] and just use array
>>   ice: pass mbxdata to ice_is_malicious_vf()
>>   ice: print message if ice_mbx_vf_state_handler returns an error
>>   ice: move ice_is_malicious_vf() to ice_virtchnl.c
>>   ice: call ice_is_malicious_vf() from ice_vc_process_vf_msg()
> 
> Everything looks legit except your anti-spamming logic which IMHO
> shouldn't happen in first place.
> 

Without the checks there's no warning to the system administrator that a
VM may have been misconfigured or modified to spam messages. If this
occurs, the VM can overload the PF's mailbox queue and prevent other VFs
from using the queue normally, and thus performing a denial of service.

My understanding (I was not involved in the original implementation or
discussions) is that there is no hardware mechanism to prevent such
overflow in this device. This is an oversight in the design which was
not caught until it was too late to make such a change.

The original checks were added in 0891c89674e8 ("ice: warn about
potentially malicious VFs"), but it seems that commit message did not
provide much detail :(

-Jake

> Thanks,
> Reviewed-by: Leon Romanovsky <leonro@...dia.com>

Powered by blists - more mailing lists