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. Martin