Am Donnerstag, dem 26.10.2023 um 09:13 -0700 schrieb Kees Cook:
> On Thu, Oct 26, 2023 at 10:15:10AM +0200, Martin Uecker wrote:
> > but not this:
> >
x->count = 11;
> > char *p = &x->buf;
> > x->count = 1;
> > p[10] = 1; // !
>
> This seems fine to me -- it's how I'd expect it to work: "10" is beyond
> "1".
Note that the store would be allowed.
>
> > (because the pointer is passed around the
> > store to the counter)
> >
> > and also here the second store is then irrelevant
> > for the access:
> >
> > x->count = 10;
> > char* p = &x->buf;
> > ...
> > x->count = 1; // somewhere else
> > ----
> > p[9] = 1; // ok, because count matter when buf was accesssed.
>
> This is less great, but I can understand why it happens. "p" loses the
> association with "x". It'd be nice if "p" had to way to retain that it
> was just an alias for x->buf, so future p access would check count.
The problem is not to discover that p is an alias to x->buf,
but that it seems difficult to make sure that stores to
x->count are not reordered relative to the final access to
p[i] you want to check, so that you then get the right value.
>
> But this appears to be an existing limitation in other areas where an
> assignment will cause the loss of object association. (I've run into
> this before.) It's just more surprising in the above example because in
> the past the loss of association would cause __bdos() to revert back to
> "SIZE_MAX" results ("I don't know the size") rather than an "outdated"
> size, which may get us into unexpected places...
>
> > IMHO this makes sense also from the user side and
> > are the desirable semantics we discussed before.
> >
> > But can you take a look at this?
> >
> >
> > This should simulate it fairly well:
> > https://godbolt.org/z/xq89aM7Gr
> >
> > (the call to the noinline function would go away,
> > but not necessarily its impact on optimization)
>
> Yeah, this example should be a very rare situation: a leaf function is
> changing the characteristics of the struct but returning a buffer within
> it to the caller. The more likely glitch would be from:
>
> int main()
> {
> struct foo *f = foo_alloc(7);
> char *p = FAM_ACCESS(f, size, buf);
>
> printf("%ld\n", __builtin_dynamic_object_size(p, 0));
> test1(f); // or just "f->count = 10;" no function call needed
> printf("%ld\n", __builtin_dynamic_object_size(p, 0));
>
> return 0;
> }
>
> which reports:
> 7
> 7
>
> instead of:
> 7
> 10
>
> This kind of "get an alias" situation is pretty common in the kernel
> as a way to have a convenient "handle" to the array. In the case of a
> "fill the array without knowing the actual final size" code pattern,
> things would immediately break:
>
> struct foo *f;
> char *p;
> int i;
>
> f = alloc(maximum_possible);
> f->count = 0;
> p = f->buf;
>
> for (i; data_is_available() && i < maximum_possible; i++) {
> f->count ++;
> p[i] = next_data_item();
> }
>
> Now perhaps the problem here is that "count" cannot be used for a count
> of "logically valid members in the array" but must always be a count of
> "allocated member space in the array", which I guess is tolerable, but
> isn't ideal -- I'd like to catch logic bugs in addition to allocation
> bugs, but the latter is certainly much more important to catch.
Maybe we could have a warning when f->buf is not directly
accessed.
Martin
>
