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: <7ab9435f-e43d-4580-b7d3-18a69f231252@intel.com>
Date: Tue, 2 Jul 2024 09:08:17 +0200
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Petr Machata <petrm@...dia.com>, Ido Schimmel <idosch@...dia.com>
CC: <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

On 7/1/24 18:41, Petr Machata wrote:
> A forgotten or buggy variable initialization can cause out-of-bounds access
> to a register or other item array field. For an overflow, such access would
> mangle adjacent parts of the register payload. For an underflow, due to all
> variables being unsigned, the access would likely trample unrelated memory.
> Since neither is correct, replace these accesses with accesses at the index
> of 0, and warn about the issue.

That is not correct either, but indeed better.

> 
> 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?

>   	be_index = max_index - index;
>   	offset = be_index * item->element_size >> 3;
>   	in_byte_index  = index % (BITS_PER_BYTE / item->element_size);