> On Oct 20, 2023, at 3:54 PM, Martin Uecker <uec...@tugraz.at> wrote:
>
> Am Freitag, dem 20.10.2023 um 18:48 +0000 schrieb Qing Zhao:
>>
>>> On Oct 20, 2023, at 2:34 PM, Kees Cook <keesc...@chromium.org> wrote:
>>>
>>> On Fri, Oct 20, 2023 at 11:50:11AM +0200, Martin Uecker wrote:
>>>> Am Donnerstag, dem 19.10.2023 um 16:33 -0700 schrieb Kees Cook:
>>>>> On Wed, Oct 18, 2023 at 09:11:43PM +0000, Qing Zhao wrote:
>>>>>> As I replied to Martin in another email, I plan to do the following to
>>>>>> resolve this issue:
>>>>>>
>>>>>> 1. No specification for signed or unsigned for counted_by field.
>>>>>> 2. Add a sanitizer option -fsanitize=counted-by-bound to catch the cases
>>>>>> when the size of the counted-by is not positive.
>>>>>
>>>>> I don't understand why this needs to be a runtime sanitizer. The
>>>>> signedness is known at compile time, so I would expect a -W option.
>>>>
>>>> The signedness of the type but not of the value.
>>>>
>>>> But I would not want to have a warning for signed
>>>> counter types by default because I would prefer
>>>> to use signed types (for various reasons including
>>>> better overflow detection).
>>>>
>>>>> Or
>>>>> do you mean you'd split up -fsanitize=bounds between unsigned and signed
>>>>> indexes? I'd find that kind of awkward for the kernel... but I feel like
>>>>> I've misunderstood something. :)
>>>>>
>>>>> -Kees
>>>>
>>>> The idea would be to detect at run-time the case
>>>> if x->buf is used at a time where x->counter
>>>> is negative and also when x->counter * sizeof(x->buf[0])
>>>> overflows or is too big.
>>>>
>>>> This would be similar to
>>>>
>>>> int a[n];
>>>>
>>>> where it is detected at run-time if n is not-positive.
>>>
>>> Right. I guess what I mean to say is that I would expect this case to
>>> already be caught by -fsanitize=bounds -- I don't see a reason to add an
>>> additional sanitizer option.
>>>
>>> struct foo {
>>> int count;
>>> int array[] __counted_by(count);
>>> };
>>>
>>> foo->count = 5;
>>> foo->array[0] = 1; // ok
>>> foo->array[10] = 1; // -fsanitize=bounds will catch this
>>> foo->array[-10] = 1; // -fsanitize=bounds will catch this too
>>>
>>>
>>
>> just checked this testing case with my GCC, and YES, -fsanitize=bounds
>> indeed caught this error:
>>
>> ttt_1.c:31:12: runtime error: index 10 out of bounds for type 'char [*]'
>> ttt_1.c:32:12: runtime error: index -10 out of bounds for type 'char [*]’
>>
>
> Yes, but I thought we were discussing the case where count is
> set to a negative value:
>
> foo->count = -1;
> int x = foo->array[3]; // UBSan should diagnose this
>
> And also the case when foo->array becomes too big.
Oops, yes, you are right.
Thanks.
Qing
>
> Martin
