[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