Skip to content

fix(tx_pool): compute tx page-pool floor in 64-bit to avoid overflow - #95

Open
randomizedcoder wants to merge 1 commit into
PlatformLab:mainfrom
randomizedcoder:fix/tx-pool-min-pages-overflow
Open

randomizedcoder wants to merge 1 commit into
PlatformLab:mainfrom
randomizedcoder:fix/tx-pool-min-pages-overflow

Conversation

@randomizedcoder

Copy link
Copy Markdown

Bug: 32-bit overflow computing the tx page-pool floor

homa_tx_pool_gc() converts the tx_page_pool_min_kb floor (in KB) into a page count:

min_pages = ((homa->tx_page_pool_min_kb * 1000)
        + (HOMA_TX_PAGE_SIZE - 1)) >> HOMA_TX_PAGE_SHIFT;

tx_page_pool_min_kb is a plain int. int * 1000 is evaluated in 32-bit, so any floor above ~2.1 GB (min_kb > 2,147,483) overflows — a multi-GB tx reserve is a perfectly legitimate sysctl setting. The overflow makes min_pages garbage, typically negative, and the routine then computes:

release = max_low_mark - min_pages;   /* negative min_pages => huge release */

so gc reclaims exactly the pages the floor was meant to protect.

Fix

-	min_pages = ((homa->tx_page_pool_min_kb * 1000)
-			+ (HOMA_TX_PAGE_SIZE - 1)) >> HOMA_TX_PAGE_SHIFT;
+	min_pages = (int)((((u64)homa->tx_page_pool_min_kb * 1000)
+			+ (HOMA_TX_PAGE_SIZE - 1)) >> HOMA_TX_PAGE_SHIFT);

Do the arithmetic in 64-bit, then narrow the (now-small) page count back to int.

Verification (TDD, red → green)

Table-driven regression test homa_tx_pool_gc__min_kb_floor_no_overflow, written first:

row floor before fix after fix
modest_floor_frees_down_to_floor ~5 pages avail 6 avail 6
floor_exceeds_pool_frees_none ~50 pages avail 10 avail 10
huge_floor_no_int_overflow 3,000,000 KB avail 0 (whole pool freed) avail 10 (kept)
Red:   Expected exp_avail (10) == pool->avail (0)   [huge_floor_no_int_overflow]
Green: homa_tx_pool_gc__min_kb_floor_no_overflow ... 1 / 1 passed
       homa_tx_pool (whole fixture) ................ 30 / 30 passed

Built and run against the test/ kselftest unit harness (ASan on).

Uses the table-driven test style proposed in the first PR of this series.

homa_tx_pool_gc() turns the tx_page_pool_min_kb floor into a page count
with `homa->tx_page_pool_min_kb * 1000`. tx_page_pool_min_kb is a plain
int, so for large floors (a multi-GB tx reserve is a legitimate sysctl
setting) the multiply overflows 32-bit int. The resulting min_pages is
garbage -- typically negative -- which makes `release = max_low_mark -
min_pages` huge, so gc reclaims pages that the floor was supposed to keep.

Promote the multiply to u64 (and cast the shifted result back to int).

Adds a table-driven regression test,
homa_tx_pool_gc__min_kb_floor_no_overflow, whose corner row sets a floor
whose `* 1000` overflows a 32-bit int: before the fix gc frees the whole
pool (avail 10 -> 0); after the fix the floor is honored and the pool is
left intact. Existing tx_pool suite stays green (30/30).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant