Re: [PATCH] Fix tcache count maximum

[Wilco Dijkstra <Wilco dot Dijkstra at arm dot com>]

Hi DJ,

>>> -  assert (tcache->counts[tc_idx] > 0);
>>
>> OK.
>
> Note I still want further justification for this one.

Well I already mentioned that all calls to tcache_get ensure there
is an entry:

  if (tc_idx < mp_.tcache_bins
      /*&& tc_idx < TCACHE_MAX_BINS*/ /* to appease gcc */
      && tcache
      && tcache->entries[tc_idx] != NULL)
    {
      return tcache_get (tc_idx);
    }

Here we've explicitly checked the linked list contains at least one
element. 

      if (return_cached
          && mp_.tcache_unsorted_limit > 0
          && tcache_unsorted_count > mp_.tcache_unsorted_limit)
        {
          return tcache_get (tc_idx);
        }

      if (return_cached)
        {
          return tcache_get (tc_idx);
        }

These cases can only call tcache_get if return_cached is true. This is set
by this code:

             /* Fill cache first, return to user only if cache fills.
                 We may return one of these chunks later.  */
              if (tcache_nb
                  && tcache->counts[tc_idx] < mp_.tcache_count)
                {
                  tcache_put (victim, tc_idx);
                  return_cached = 1;
                  continue;
                }

Now it is of course feasible to overwrite the tcace count or the entries or corrupt
the blocks held in the tcache list. If that happens then all bets are off, since any
targeted corruption can be made to look like a valid entry. This is true for all the
malloc datastructures - you need to encrypt all the fields to reduce the chances
of being able to spoof the fields, but that is way too much overhead.

Note that these asserts are trivially redundant too:

static __always_inline void
tcache_put (mchunkptr chunk, size_t tc_idx)
{
  tcache_entry *e = (tcache_entry *) chunk2mem (chunk);
  assert (tc_idx < TCACHE_MAX_BINS);
...

static __always_inline void *
tcache_get (size_t tc_idx)
{
  tcache_entry *e = tcache->entries[tc_idx];
  assert (tc_idx < TCACHE_MAX_BINS);

Adding these asserts just makes the tcache slower without making it any safer.

Wilco