MEDIUM: pools: refine pool size rounding
authorWilly Tarreau <w@1wt.eu>
Tue, 12 Sep 2023 13:38:32 +0000 (15:38 +0200)
committerWilly Tarreau <w@1wt.eu>
Tue, 12 Sep 2023 16:14:05 +0000 (18:14 +0200)
commit93c2ea0ec3cdc9b5592ba09738ccef3dd0500c54
tree7f13bac41f475971c1b25c9b807c1dadd36f6834
parent61575769acc6b117f31d5bdb1bafc4d5da13b5d4
MEDIUM: pools: refine pool size rounding

The pools sizes were rounded up a little bit too much with commit
30f931ead ("BUG/MEDIUM: pools: fix the minimum allocation size"). The
goal was in fact to make sure they were always at least large enough to
store 2 list heads, and stuffing this into the alignment calculation
resulted in the size being always rounded up to this size. This is
problematic because it means that the appended tag at the end doesn't
always catch potential overflows since more bytes than needed are
allocated. Moreover, this test was later reinforced by commit b5ba09ed5
("BUG/MEDIUM: pools: ensure items are always large enough for the
pool_cache_item"), proving that the first test was not always sufficient.

This needs to be reworked to proceed correctly:
  - the two lists are needed when the object is in the cache, hence
    when we don't care about the tag, which means that the tag's size,
    if any, can easily cover for the missing bytes to reach that size.
    This is actually what was already being checked for.

  - the rounding should not be performed (beyond the size of a word to
    preserve pointer alignment) when pool tagging is enabled, otherwise
    we don't detect small overflows. It means that there will be less
    merging when proceeding like this. Tests show that we merge 93 pools
    into 36 without tags and 43 with tags enabled.

  - the rounding should not consider the extra size, since it's already
    done when calculating the allocated size later (i.e. don't round up
    twice). The difference is subtle but it's what makes sure the tag
    immediately follows the area instead of starting from the end.

Thanks to this, now when writing one byte too many at the end of a struct
stream, the error is instantly caught.
src/pool.c