Re: [PATCH] Update tcache double-free check

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

Hello Eyal,

Carlos asked me to take a look at this because we think it's important 
to close this hole if we can.  Sorry we took long to get back to this.

On 8/10/20 7:05 PM, Eyal Itkin via Libc-alpha wrote:
> Due to that, I recommend using a random value per process according to
> the original scheme of getrandom() and a backup of "~tcache". Keeping
> in mind that the ASLR bits will supply some random, and that ~tcache
> will never be a value we expect to see in memory and won't ever be a
> mapped memory address that could be used by a use-after-free
> vulnerability as the original key ("tcache") was exploited. I don't
> see any value in a per-thread random key, as again, the random value
> will only help us avoid collisions with values from the program's
> memory.

Agreed, a per-process key is sufficient.  However your latest patch 
appears to have a per-thread key; I assume you intend to post an update 
patch based on feedback?

Given that our only goals are (1) put in some random value to avoid 
collisions and (2) not leak the address of an internal structure in the 
process, could this just be a single process-wide variable that is 
initialized at startup?

What this implies is that tcache_key (a static variable and not 
__thread) is initialized in ptmalloc_init instead of tcache_init and the 
same value is used in all threads.  ptmalloc_init is guaranteed to run 
in a single-threaded context as malloc gets called in the process of 
thread creation before it is spawned and hence should not need any 
synchronization.

Finally for randomness of the tcache_key, it might make sense to use 
getrandom first (as the patch already does) and then on failure, fall 
back to a munging of ~tcache and random_bits() (from 
include/random-bits.h) so that we don't leak addresses even on fallback. 
  However I'm not convinced that getrandom() is strictly necessary given 
our goals; I reckon it's just harmless given that it's a one time cost.

Siddhesh

PS: I'll make sure you're cc'd on this conversation, so you don't have 
to be registered to the mailing list to stay on track with this 
conversation.