[PATCH] malloc tcache: Debugger now sees the address of the corrupted chunk.

DJ Delorie dj@redhat.com
Wed Dec 16 22:30:44 GMT 2020


Adder <adder.thief@gmail.com> writes:

> And shuffling the qualifier eliminates the need for casting to void.

The (void) is a common idiom; we should leave it even if it has no
effect *at the moment*.  It tells future code readers our current
intentions, and is more resilient to compiler changes as the compiler
devs know about it too.

> We can also use the following:
>
>   ((volatile tcache_entry *) tcache->entries[tc_idx])->next;
>
> It produces the exact same machine code, but it is more resilient
> to changes in the definition of our tcache_entry struct
> (example: to a field being added before our first field, i.e. before "next").

For validating the "next" pointer, it doesn't matter what the "next"
tcache_entry struct looks like; it only matters if it's in valid memory.
Reading the first word of the entry is sufficient.  tcache->entries[] is
a pointer, e->next is a pointer, either way we need to read *one* word
to validate that pointer.  We don't need to read the whole struct it
points to, nor does it matter which word in the struct is read.

I would argue that casting to a non-struct pointer emphasizes that we
care about the pointer more than what it points to, but a sufficiently
verbose comment would suffice as well.

Whether we use e->next or tcache->entries for this purpose is
irrelevent, I think.  Whichever results in a smaller line of code is
probably a bit cleaner ;-)

>     mov [r9], rsi ; Crash here if e happens to be an only-allow-reading pointer.

Given the size of a 64-bit address space, the odds of randomly hitting a
read-only page is negligible, compared to the odds of hitting an
unmapped page, which is very high.  Given that this code is on the fast
path, I would argue against this as "not useful enough".

> +  if (__glibc_unlikely (e->key != tcache))
> +    * ((tcache_entry *volatile *) 0) = e;

This is where malloc_printerr should be called.  Even if the data is
corrupt, this is what we do elsewhere in these cases.

> I wish to suggest that we quarrel on Comments after we agree on Code. (-:

Perhaps, but good comments will be required ;-)



More information about the Libc-alpha mailing list