On Thu, Oct 26, 2023 at 10:15:10AM +0200, Martin Uecker wrote:
> but not this:
>
> 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".
> (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.
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.
--
Kees Cook
