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
--
Kees Cook
