[PATCH v4 2/7] malloc: mangle tcache entries pointers

[Cupertino Miranda]

Hi DJ,

My thought on protecting the entries was to simplify the conditioning 
within tcache_get_n function.

-  tcache_entry *e;
-  if (ep == &(tcache->entries[tc_idx]))
-    e = *ep;
-  else
-    e = REVEAL_PTR (*ep);
+  tcache_entry *e = REVEAL_PTR (*ep);
...
-  if (ep == &(tcache->entries[tc_idx]))
-      *ep = REVEAL_PTR (e->next);
-  else
-    *ep = PROTECT_PTR (ep, REVEAL_PTR (e->next));
+  *ep = PROTECT_PTR (ep, REVEAL_PTR (e->next));

This might not be so required in the initial single-size chunk bins, 
since you would always get the chunk for the head of the bin, not 
needing to traverse the bin.
On the other hand with the content of patch 4, if we so decide to change 
standard behavior with the new tunable, then it would have some impact.

Please notice the very simple (fewer conditions) implementation of 
tcache_location_for_size within patch 4.
This function would find the position to any sized bins in both fixed 
and range size tcache bins.

On 14-03-2025 23:47, DJ Delorie wrote:
> Cupertino Miranda <cupertino.miranda@oracle.com> writes:
>> -      while (tcache_tmp->entries[i])
>> +      while (REVEAL_PTR (tcache_tmp->entries[i]))
> 
> Do we really want to protect the NULL entries?
At least in some locations we would require extra conditioning.
Maybe it is not so bad.

> Doing that makes it much
> easier for an attacker to get the protection key, as most entries would
> be protected NULL at first.  (although the key is just the address of
> the entries array itself, so I guess you'd already have some way to get it).
Not an expert in this type of protections, but to me it seems more of an 
obfuscation such that an attacker would not easily deduce the data 
refers to a linked list. Once someone figures out that, it would be 
rather trivial to patch the linked-list in any case, IMHO.
> 
> The patch otherwise looks OK to me though.
> Reviewed-by: DJ Delorie <dj@redhat.com>
>