Skip to content

uucore: fix parse_size overflow on a large % size - #13742

Open
MsfPablo wants to merge 1 commit into
uutils:mainfrom
MsfPablo:fix/parse-size-percent-overflow
Open

uucore: fix parse_size overflow on a large % size#13742
MsfPablo wants to merge 1 commit into
uutils:mainfrom
MsfPablo:fix/parse-size-percent-overflow

Conversation

@MsfPablo

@MsfPablo MsfPablo commented Aug 4, 2026

Copy link
Copy Markdown

Summary

Fixes #13736. The % (percent-of-physical-memory) branch of Parser::parse in src/uucore/src/lib/features/parser/parse_size.rs computed (number / 100) * total as an unchecked u128 multiply. A percentage that fits in a u128 but whose product with the total physical memory does not overflows, panicking with "attempt to multiply with overflow" (exit 134) under -C overflow-checks instead of reporting a parse error:

$ du --block-size=9223372036854775808000000000000% /tmp
thread 'main' panicked at parse_size.rs:258:33:
attempt to multiply with overflow

Since parse_size is shared code, this is reachable from every util that accepts a size: du, sort, df, ls, split, stdbuf, od, shred, dd, head, and tail.

Fix

Use checked_mul and return ParseSizeError::size_too_big on overflow, matching how the non-percent path (a few lines below) already handles the same too-large-size condition — per @RenjiSann's comment on the issue. Callers get the same behavior they already have for other too-large sizes like 1Y: utils that report an error keep reporting one, and utils using parse_u64_max/parse_u128_max clamp to the maximum, matching GNU.

Test plan

Added a regression test (parse_percent_overflow) covering the exact overflow case, gated #[cfg(target_os = "linux")] to match the existing convention for physical-memory-dependent tests in this file (there are several already gated the same way, since total_physical_memory() is platform-specific).

  • cargo test -p uucore --lib --features parser-size — 16/16 existing parse_size tests pass (the new Linux-gated test doesn't run on my macOS sandbox, but follows the exact same gating as pre-existing tests in this file, so it'll run in CI)
  • cargo clippy -p uucore --features parser-size --lib -- -D warnings — clean

The `%` (percent-of-physical-memory) branch of `Parser::parse` computed
`(number / 100) * total` as an unchecked `u128` multiply. A percentage
that fits in a `u128` but whose product with the total physical memory
does not would overflow, panicking with "attempt to multiply with
overflow" under `-C overflow-checks` (exit 134) instead of reporting a
parse error:

    $ du --block-size=9223372036854775808000000000000% /tmp
    thread 'main' panicked at parse_size.rs:258:33:
    attempt to multiply with overflow

Because `parse_size` is shared code, the panic was reachable from every
util that accepts a size: du, sort, df, ls, split, stdbuf, od, shred,
dd, head and tail.

Use `checked_mul` and return `ParseSizeError::SizeTooBig` on overflow,
matching how the non-percent path already handles the same condition a
few lines below. Callers therefore get the behaviour they already have
for other too-large sizes such as `1Y`: utils that report an error keep
reporting one, and utils using `parse_u64_max`/`parse_u128_max` clamp to
the maximum, as GNU does.

Fixes uutils#13736
@codspeed-hq

codspeed-hq Bot commented Aug 4, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 18.46%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

❌ 1 regressed benchmark
✅ 346 untouched benchmarks
⏩ 46 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation df_with_path 571 µs 700.2 µs -18.46%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing MsfPablo:fix/parse-size-percent-overflow (44f2b0b) with main (0b214cc)

Open in CodSpeed

Footnotes

  1. 46 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@sylvestre

Copy link
Copy Markdown
Contributor

please add tests in the programs to make sure we don't regress, thanks

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

GNU testsuite comparison:

GNU test failed: tests/misc/sync. tests/misc/sync is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/cut/bounded-memory (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/tail-n0f (fails in this run but passes in the 'main' branch)
Note: The gnu test tests/misc/write-errors was skipped on 'main' but is now failing.

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.

uucore parse_size: arithmetic overflow (overflow-checks) on a large % (percent-of-memory) size

2 participants