netdev - Re: [PATCH net-next 1/3] mlxsw: Warn about invalid accesses to array fields

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: <87le2ctdwb.fsf@nvidia.com>
Date: Mon, 8 Jul 2024 11:45:03 +0200
From: Petr Machata <petrm@...dia.com>
To: Ido Schimmel <idosch@...dia.com>
CC: Przemek Kitszel <przemyslaw.kitszel@...el.com>, Petr Machata
	<petrm@...dia.com>, <mlxsw@...dia.com>, "David S. Miller"
	<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
	<kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 1/3] mlxsw: Warn about invalid accesses to
 array fields


Ido Schimmel <idosch@...dia.com> writes:

> On Tue, Jul 02, 2024 at 09:08:17AM +0200, Przemek Kitszel wrote:
>> On 7/1/24 18:41, Petr Machata wrote:
>> > 
>> > Suggested-by: Ido Schimmel <idosch@...dia.com>
>> > Signed-off-by: Petr Machata <petrm@...dia.com>
>> > Reviewed-by: Ido Schimmel <idosch@...dia.com>
>> > ---
>> >   drivers/net/ethernet/mellanox/mlxsw/item.h | 2 ++
>> >   1 file changed, 2 insertions(+)
>> > 
>> > diff --git a/drivers/net/ethernet/mellanox/mlxsw/item.h b/drivers/net/ethernet/mellanox/mlxsw/item.h
>> > index cfafbeb42586..9f7133735760 100644
>> > --- a/drivers/net/ethernet/mellanox/mlxsw/item.h
>> > +++ b/drivers/net/ethernet/mellanox/mlxsw/item.h
>> > @@ -218,6 +218,8 @@ __mlxsw_item_bit_array_offset(const struct mlxsw_item *item,
>> >   	}
>> >   	max_index = (item->size.bytes << 3) / item->element_size - 1;
>> > +	if (WARN_ON(index > max_index))
>> > +		index = 0;
>> 
>> you have BUG*() calls just above those lines :(
>> anyway, WARN_ON_ONCE(), and perhaps you need to print some additional
>> data to finally fix this?
>
> The trace should be enough, but more info can be added:
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/item.h b/drivers/net/ethernet/mellanox/mlxsw/item.h
> index 9f7133735760..a619a0736bd1 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/item.h
> +++ b/drivers/net/ethernet/mellanox/mlxsw/item.h
> @@ -218,7 +218,9 @@ __mlxsw_item_bit_array_offset(const struct mlxsw_item *item,
>         }
>  
>         max_index = (item->size.bytes << 3) / item->element_size - 1;
> -       if (WARN_ON(index > max_index))
> +       if (WARN_ONCE(index > max_index,
> +                     "name=%s,index=%u,max_index=%u\n", item->name, index,
> +                     max_index))
>                 index = 0;
>         be_index = max_index - index;
>         offset = be_index * item->element_size >> 3;
>
> Will leave it to Petr to decide what he wants to include there.

Thanks, I'll send a v2.