> On Jun 18, 2025, at 12:17, Siddhesh Poyarekar <siddh...@gotplt.org> wrote:
>
> 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.
Okay, I guess that I didn’t put enough attention on the above example
previously, sorry about that...
Read it multiple times this time, my question is for the following code
portion:
objsz = __builtin_dynamic_object_size (ptr, 0);
__memcpy_chk (ptr, src, sz, objsz);
When program get to the this point, “ptr” is freed and invalid already, is
the program still considered as a valid program when the first argument to the
call to __memcpy_chk is an invalid pointer but the 3rd parameter is 0?
Qing
>
> I hope that clarifies what I'm trying to say.
>
> Sid
