linux-kernel - Re: [PATCH net 1/2] mlxsw: spectrum_acl_bloom_filter: Expand chunk_key_offsets[chunk

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: <8563032ED2B6B840+7af17f62-992a-4275-80c7-ac7ef5276ae7@uniontech.com>
Date: Fri, 14 Mar 2025 00:10:42 +0800
From: WangYuli <wangyuli@...ontech.com>
To: Ido Schimmel <idosch@...dia.com>, Paolo Abeni <pabeni@...hat.com>
Cc: andrew+netdev@...n.ch, chenlinxuan@...ontech.com, czj2441@....com,
 davem@...emloft.net, edumazet@...gle.com, guanwentao@...ontech.com,
 kuba@...nel.org, linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
 niecheng1@...ontech.com, petrm@...dia.com, zhanjun@...ontech.com
Subject: Re: [PATCH net 1/2] mlxsw: spectrum_acl_bloom_filter: Expand
 chunk_key_offsets[chunk_index]

Hi Ido Schimmel,

On 2025/3/13 23:25, Ido Schimmel wrote:
> I would like to get a warning if 'chunk_count' is larger than
> 'MLXSW_BLOOM_KEY_CHUNKS' since it should never happen.
>
> I was able to reproduce the build failure and the following patch seems
> to solve it for me. It removes the 'max_chunks' argument since it's
> always 'MLXSW_BLOOM_KEY_CHUNKS' (3) and verifies that the number of
> chunks that was calculated is never greater than it.
>
> WangYuli, can you please test it?

My tests still show the same compilation failing.

Indeed, I have already tried similar fixes during my investigation, but 
to no avail.

> Also, if you want it in net.git (as opposed to net-next.git), then it
> needs a Fixes tag:
>
> Fixes: 7585cacdb978 ("mlxsw: spectrum_acl: Add Bloom filter handling")
OK
>
> And I don't think we need patch #2.
While #2 is admittedly a secondary modification compared to #1, should 
you be agreeable, I believe it's still preferable to combine #2 with #1 
instead of outright discarding #2.
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> index a54eedb69a3f..a1ab5b09a6c5 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> @@ -231,7 +231,7 @@ static u16 mlxsw_sp2_acl_bf_crc(const u8 *buffer, size_t len)
>   static void
>   __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
>   			     struct mlxsw_sp_acl_atcam_entry *aentry,
> -			     char *output, u8 *len, u8 max_chunks, u8 pad_bytes,
> +			     char *output, u8 *len, u8 pad_bytes,
>   			     u8 key_offset, u8 chunk_key_len, u8 chunk_len)
>   {
>   	struct mlxsw_afk_key_info *key_info = aregion->region->key_info;
> @@ -241,10 +241,14 @@ __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
>   
>   	block_count = mlxsw_afk_key_info_blocks_count_get(key_info);
>   	chunk_count = 1 + ((block_count - 1) >> 2);
> +	if (WARN_ON_ONCE(chunk_count > MLXSW_BLOOM_KEY_CHUNKS)) {
> +		*len = 0;
> +		return;
> +	}
>   	erp_region_id = cpu_to_be16(aentry->ht_key.erp_id |
>   				   (aregion->region->id << 4));
> -	for (chunk_index = max_chunks - chunk_count; chunk_index < max_chunks;
> -	     chunk_index++) {
> +	for (chunk_index = MLXSW_BLOOM_KEY_CHUNKS - chunk_count;
> +	     chunk_index < MLXSW_BLOOM_KEY_CHUNKS; chunk_index++) {
>   		memset(chunk, 0, pad_bytes);
>   		memcpy(chunk + pad_bytes, &erp_region_id,
>   		       sizeof(erp_region_id));
> @@ -262,7 +266,6 @@ mlxsw_sp2_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
>   			    char *output, u8 *len)
>   {
>   	__mlxsw_sp_acl_bf_key_encode(aregion, aentry, output, len,
> -				     MLXSW_BLOOM_KEY_CHUNKS,
>   				     MLXSW_SP2_BLOOM_CHUNK_PAD_BYTES,
>   				     MLXSW_SP2_BLOOM_CHUNK_KEY_OFFSET,
>   				     MLXSW_SP2_BLOOM_CHUNK_KEY_BYTES,
> @@ -379,7 +382,6 @@ mlxsw_sp4_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
>   	u8 chunk_count = 1 + ((block_count - 1) >> 2);
>   
>   	__mlxsw_sp_acl_bf_key_encode(aregion, aentry, output, len,
> -				     MLXSW_BLOOM_KEY_CHUNKS,
>   				     MLXSW_SP4_BLOOM_CHUNK_PAD_BYTES,
>   				     MLXSW_SP4_BLOOM_CHUNK_KEY_OFFSET,
>   				     MLXSW_SP4_BLOOM_CHUNK_KEY_BYTES,
>
-- 
WangYuli

Content of type "text/html" skipped

Download attachment "OpenPGP_0xC5DA1F3046F40BEE.asc" of type "application/pgp-keys" (633 bytes)

Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (237 bytes)