On 2025-06-18 10:32, Qing Zhao wrote:
A NULL check alone does not solve this problem because the pointer could be
arbitrary and invalid. I thought some more about adding the size expression
only if there's an existing access like I suggested in that email, but I
realize that doesn't make sense either, since code that uses __bdos typically
would look to the __bdos result to determine validity of the access.
Can you see a way to validate the pointer in addition to the NULL check?
Right now, we only generate the size expression for the following cases:
1. When the pointer is read as the first parameter of the
__builtin_dynamic_object_size (p, 0);
AND
2. p != NULL.
Is the above 1 & 2 not enough to avoid the introduced undefined behavior?
I thought that the above 1 should validate the pointer already, let me know if
this is not the case.
If So, could you please provide me a small testing case that illustrates the
issue?
I'm afraid not, a NULL pointer is not the only case where a pointer
could be invalid. For example, the pointer could have been recently
freed or even be an arbitrary non-NULL value due to a memory corruption.
Lastly, the memory location could be protected (e.g. PROT_NONE) which
makes reading it an invalid operation.
For example if you have:
struct fam_container
{
int a;
int count;
char fam[] __counted_by__ (count);
};
void
baz (struct fam_container *ptr)
{
...
ptr->count = 0;
__builtin_free (ptr);
...
}
void
bar (size_t insz)
{
struct fam_container *c =
__builtin_malloc (insz + sizeof (struct fam_container));
c->count = insz;
baz (c);
foo (c);
}
void
__attribute__ ((noinline))
foo (struct fam_container *ptr)
{
...
objsz = __builtin_dynamic_object_size (ptr, 0);
__memcpy_chk (ptr, src, sz, objsz);
...
}
The function foo would transform to something like:
void
foo (struct fam_container *ptr)
{
...
objsz = ptr != NULL ? ptr->count + sizeof (*ptr) : -1;
__memcpy_chk (ptr, src, sz, objsz);
}
Here, ptr is not NULL and tree-object-size ends up generating a
dereference of a freed pointer, thus generating new undefined behaviour.
Earlier too the program was buggy, i.e. __memcpy_chk was being passed
a freed PTR, but tree-object-size ends up adding yet another invalid
access BEFORE that one.
The example above is a simplification of how _FORTIFY_SOURCE works, i.e.
__bdos is passed as an additional size argument to a checking function
before the actual libc function is called. Essentially, the __bdos is
supposed to protect access to PTR and instead, ends up adding another
invalid access to PTR. Admittedly, _FORTIFY_SOURCE is unable to protect
pointer corruption, but at least it should not generate a new invalid
dereference.
To demonstrate how it could make things objectively worse, consider this
example:
struct fam_container
{
int a;
int count;
char fam[] __counted_by__ (count);
};
size_t
baz (struct fam_container *ptr)
{
...
ptr->count = 0;
__builtin_free (ptr);
...
return 0;
}
void
bar (size_t insz)
{
struct fam_container *c =
__builtin_malloc (insz + sizeof (struct fam_container));
c->count = insz;
sz = baz (c);
foo (c, sz);
}
void
__attribute__ ((noinline))
foo (struct fam_container *ptr, size_t sz)
{
...
objsz = __builtin_dynamic_object_size (ptr, 0);
__memcpy_chk (ptr, src, sz, objsz);
...
}
Because SZ is 0, in practice there would have been no dereference of PTR
in __memcpy_chk because the underlying memcpy would have not
dereferenced PTR. Here, _FORTIFY_SOURCE ends up actually introducing an
invalid dereference through the OBJSZ argument.
I hope that clarifies what I'm trying to say.
Sid
