[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20251027122435.22442-1-enjuk@amazon.com>
Date: Mon, 27 Oct 2025 21:24:21 +0900
From: Kohei Enju <enjuk@...zon.com>
To: <przemyslaw.kitszel@...el.com>
CC: <andrew+netdev@...n.ch>, <anthony.l.nguyen@...el.com>,
<davem@...emloft.net>, <edumazet@...gle.com>, <enjuk@...zon.com>,
<intel-wired-lan@...ts.osuosl.org>, <kohei.enju@...il.com>,
<kuba@...nel.org>, <mitch.a.williams@...el.com>, <netdev@...r.kernel.org>,
<pabeni@...hat.com>
Subject: Re: [Intel-wired-lan] [PATCH iwl-net v1] iavf: fix off-by-one issues in iavf_config_rss_reg()
On Mon, 27 Oct 2025 13:13:54 +0100, Przemek Kitszel wrote:
>On 10/25/25 18:58, Kohei Enju wrote:
>> There are off-by-one bugs when configuring RSS hash key and lookup
>> table, causing out-of-bounds reads to memory [1] and out-of-bounds
>> writes to device registers.
>>
>> Before commit 43a3d9ba34c9 ("i40evf: Allow PF driver to configure RSS"),
>> the loop upper bounds were:
>> i <= I40E_VFQF_{HKEY,HLUT}_MAX_INDEX
>> which is safe since the value is the last valid index.
>>
>> That commit changed the bounds to:
>> i <= adapter->rss_{key,lut}_size / 4
>> where `rss_{key,lut}_size / 4` is the number of dwords, so the last
>> valid index is `(rss_{key,lut}_size / 4) - 1`. Therefore, using `<=`
>> accesses one element past the end.
>>
>> Fix the issues by using `<` instead of `<=`, ensuring we do not exceed
>> the bounds.
>>
>> [1] KASAN splat about rss_key_size off-by-one
>> BUG: KASAN: slab-out-of-bounds in iavf_config_rss+0x619/0x800
>> Read of size 4 at addr ffff888102c50134 by task kworker/u8:6/63
>>
>
>[...]
>
>>
>> Fixes: 43a3d9ba34c9 ("i40evf: Allow PF driver to configure RSS")
>> Signed-off-by: Kohei Enju <enjuk@...zon.com>
>> ---
>> drivers/net/ethernet/intel/iavf/iavf_main.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
>> index c2fbe443ef85..4b0fc8f354bc 100644
>> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
>> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
>> @@ -1726,11 +1726,11 @@ static int iavf_config_rss_reg(struct iavf_adapter *adapter)
>> u16 i;
>>
>> dw = (u32 *)adapter->rss_key;
>> - for (i = 0; i <= adapter->rss_key_size / 4; i++)
>> + for (i = 0; i < adapter->rss_key_size / 4; i++)
>> wr32(hw, IAVF_VFQF_HKEY(i), dw[i]);
>>
>> dw = (u32 *)adapter->rss_lut;
>> - for (i = 0; i <= adapter->rss_lut_size / 4; i++)
>> + for (i = 0; i < adapter->rss_lut_size / 4; i++)
>> wr32(hw, IAVF_VFQF_HLUT(i), dw[i]);
>
>this is generally the last defined register mapping,
>so I get why KASAN is able to report a violation here
>(I assume that we map "just enough")
Just to clarify, I think KASAN is detecting OOB read access to the slab
memory region (dw[i]), and not detecting register write access (wr32())
directly.
Anyway, thank you for reviewing, Przemek!
>
>impressive, and thanks for the fix!
>
>Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
>
>>
>> iavf_flush(hw);
>
>
Powered by blists - more mailing lists