netdev - Re: [ovs-dev] [PATCH net-next 2/2] net: openvswitch: Annotate struct mask_array with __counted_byUse struct

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] [day] [month] [year] [list]

Date: Tue, 10 Oct 2023 07:27:48 +0200
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: Ilya Maximets <i.maximets@....org>, keescook@...omium.org,
 Pravin B Shelar <pshelar@....org>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, "Gustavo A. R. Silva"
 <gustavoars@...nel.org>, Nathan Chancellor <nathan@...nel.org>,
 Nick Desaulniers <ndesaulniers@...gle.com>, Tom Rix <trix@...hat.com>
Cc: dev@...nvswitch.org, netdev@...r.kernel.org, llvm@...ts.linux.dev,
 kernel-janitors@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-hardening@...r.kernel.org
Subject: Re: [ovs-dev] [PATCH net-next 2/2] net: openvswitch: Annotate struct
 mask_array with __counted_byUse struct_size()

Le 02/10/2023 à 18:51, Ilya Maximets a écrit :
> On 10/1/23 13:07, Christophe JAILLET wrote:
>> Prepare for the coming implementation by GCC and Clang of the __counted_by
>> attribute. Flexible array members annotated with __counted_by can have
>> their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
>> (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
>> functions).
>>
>> Signed-off-by: Christophe JAILLET <christophe.jaillet@...adoo.fr>
>> ---
>> This patch is part of a work done in parallel of what is currently worked
>> on by Kees Cook.
>>
>> My patches are only related to corner cases that do NOT match the
>> semantic of his Coccinelle script[1].
>>
>> In this case, in tbl_mask_array_alloc(), several things are allocated with
>> a single allocation. Then, some pointer arithmetic computes the address of
>> the memory after the flex-array.
>>
>> [1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
>> ---
>>   net/openvswitch/flow_table.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/openvswitch/flow_table.h b/net/openvswitch/flow_table.h
>> index 9e659db78c05..8d9e83b4d62c 100644
>> --- a/net/openvswitch/flow_table.h
>> +++ b/net/openvswitch/flow_table.h
>> @@ -48,7 +48,7 @@ struct mask_array {
>>   	int count, max;
>>   	struct mask_array_stats __percpu *masks_usage_stats;
>>   	u64 *masks_usage_zero_cntr;
>> -	struct sw_flow_mask __rcu *masks[];
>> +	struct sw_flow_mask __rcu *masks[] __counted_by(size);
> 
> Did you mean 'max'?  There is no 'size' in the structure.

Hi,

Of courtse, yes. I'll resend.

'size' is the name of the variable that is written in mask_array->max in 
tbl_mask_array_alloc()

> 
> Also, the patch subject is messed up a bit.

Yes.
Will fix it as well.

CJ

> 
> Best regards, Ilya Maximets.
>