Skip to content

xargs: cap single-argument growth to avoid OOM on unterminated input - #820

Open
MsfPablo wants to merge 3 commits into
uutils:mainfrom
MsfPablo:fix/xargs-devfull-oom-770
Open

xargs: cap single-argument growth to avoid OOM on unterminated input#820
MsfPablo wants to merge 3 commits into
uutils:mainfrom
MsfPablo:fix/xargs-devfull-oom-770

Conversation

@MsfPablo

@MsfPablo MsfPablo commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Closes #770.

Problem

xargs reading from an input that never produces a delimiter — most notably /dev/full, which yields an endless stream of NUL bytes — accumulates a single argument without bound until the process is OOM-killed. Nothing bounded argument growth between delimiters, so a terminator-less (or non-matching-delimiter) stream could grow the reader buffer indefinitely.

A minimal repro (Linux, with /dev/full):

$ xargs -a /dev/full echo
# ... accumulates memory until OOM-killed

Fix

Bound each accumulated argument by a multiple of the effective command-line character budget — the same budget the -s / system ARG_MAX-derived limiters already enforce — and report argument line too long once an argument exceeds it. This mirrors the error GNU xargs produces in this situation.

  • New MaxCharsCommandSizeLimiter::effective_max_chars(options_max_chars, env) returns the per-command-line budget (min of user -s and the system limit, or the 128 KiB default), used both to construct the existing limiter and to derive the argument cap.
  • ARG_SIZE_OVERFLOW_MULTIPLIER = 4: the cap is generously sized so that no argument the size limiters would ever accept can be rejected by the reader — it only ever trips on truly unbounded growth.
  • Both WhitespaceDelimitedArgumentReader and ByteDelimitedArgumentReader gain a max_arg_size bound, checked as bytes are accumulated.
  • The reader error type is widened from io::Result to a small XargsError enum so the argument line too long condition is distinguishable from an ordinary I/O error (e.g. Interrupted, BrokenPipe).

The byte-delimited reader previously relied on BufReader::read_until to loop internally; driving the buffered reader by hand (so the cap can be checked) surfaces Interrupted reads to the caller, so those are retried explicitly to preserve the previous behaviour.

Verification

  • cargo build — clean
  • cargo clippy --all-targets -- -D warnings — clean
  • cargo fmt --check — clean
  • cargo test — 360 tests pass, 0 failures

Added two unit tests covering the cap directly (portable, no /dev/full dependency) so the behaviour is locked in regardless of platform:

  • test_whitespace_reader_caps_unbounded_argument — a long run of non-whitespace with no terminator returns ArgumentTooLarge instead of looping forever.
  • test_byte_reader_caps_unbounded_argument — a stream lacking the delimiter byte returns ArgumentTooLarge instead of looping forever.

Existing reader tests were updated to the new constructor signatures and the XargsError return type; the Interrupted-retried and BrokenPipe-propagation behaviours are still covered by test_byte_delimited_reader and test_eof_argument_reader.

/dev/full itself is Linux-only, so I kept the end-to-end repro on Linux rather than gating a test behind cfg(target_os = "linux"); the unit tests above exercise the same code path portably.

@codecov

codecov Bot commented Aug 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.29730% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.23%. Comparing base (2a3eac9) to head (aefc1d0).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/xargs/mod.rs 97.29% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #820      +/-   ##
==========================================
+ Coverage   92.15%   92.23%   +0.08%     
==========================================
  Files          35       35              
  Lines        7377     7486     +109     
  Branches      383      391       +8     
==========================================
+ Hits         6798     6905     +107     
- Misses        438      439       +1     
- Partials      141      142       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codspeed-hq

codspeed-hq Bot commented Aug 8, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 20 untouched benchmarks


Comparing MsfPablo:fix/xargs-devfull-oom-770 (aefc1d0) with main (1acf40a)

Open in CodSpeed

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown

Commit 44f265b has test result changes:

bfs testsuite:

Test results comparison:
  Current:   TOTAL: 314 / PASSED: 267 / FAILED: 41 / SKIPPED: 6
  Reference: TOTAL: 312 / PASSED: 266 / FAILED: 40 / SKIPPED: 6

Changes from main branch:
  TOTAL: +2
  PASSED: +1
  FAILED: +1

New test failures (1):
  - gnu/okdir_path_empty

@sylvestre

Copy link
Copy Markdown
Contributor

please add a test in test_find

@github-actions

Copy link
Copy Markdown

Commit 55721e8 has test result changes:

bfs testsuite:

Test results comparison:
  Current:   TOTAL: 314 / PASSED: 267 / FAILED: 41 / SKIPPED: 6
  Reference: TOTAL: 313 / PASSED: 267 / FAILED: 40 / SKIPPED: 6

Changes from main branch:
  TOTAL: +1
  PASSED: +0
  FAILED: +1

New test failures (1):
  - gnu/files0_from_ok

@sylvestre

Copy link
Copy Markdown
Contributor

jobs are failing

contributor and others added 2 commits August 18, 2026 19:22
Reading from an input that never produces a delimiter (such as
`/dev/full`, which yields an endless stream of NUL bytes) made the
argument readers accumulate a single argument without bound until the
process was OOM-killed, because nothing bounded the growth between
delimiters.

Bound each accumulated argument by a multiple of the effective
command-line character budget (the same budget the `-s` / system
ARG_MAX limiters enforce), reporting `argument line too long` once an
argument exceeds it. The cap is sized generously (4x the budget) so that
no argument the size limiters would ever accept can be rejected by the
reader.

The byte-delimited reader previously relied on `BufReader::read_until`
to loop internally; driving the buffered reader by hand (so the cap can
be checked) surfaces `Interrupted` reads to the caller, so retry those
explicitly to preserve the previous behaviour.
@sylvestre
sylvestre force-pushed the fix/xargs-devfull-oom-770 branch from 55721e8 to 30bade4 Compare August 18, 2026 17:22
@github-actions

Copy link
Copy Markdown

Commit 30bade4 has test result changes:

bfs testsuite:

Test results comparison:
  Current:   TOTAL: 317 / PASSED: 274 / FAILED: 37 / SKIPPED: 6
  Reference: TOTAL: 313 / PASSED: 267 / FAILED: 40 / SKIPPED: 6

Changes from main branch:
  TOTAL: +4
  PASSED: +7
  FAILED: -3

Test improvements (3):
  + gnu/execdir_path_dot
  + gnu/execdir_path_empty
  + gnu/execdir_path_relative

The OOM cap commit (b60b1c6) added a max_arg_size parameter to
WhitespaceDelimitedArgumentReader::new, but six lib-test call sites
still constructed the reader with one argument, so `cargo clippy -- -D
warnings` failed with E0061. Pass usize::MAX at those sites — they
exercise the reader's tokenization, not the cap.
@github-actions

Copy link
Copy Markdown

Commit aefc1d0 has test result changes:

GNU findutils testsuite:

Test results comparison:
  Current:   TOTAL: 495 / PASSED: 421 / FAILED: 73 / SKIPPED: 1
  Reference: TOTAL: 495 / PASSED: 420 / FAILED: 74 / SKIPPED: 1

Changes from main branch:
  TOTAL: +0
  PASSED: +1
  FAILED: -1

Test improvements (1):
  + size-invalid.new-O3

bfs testsuite:

Test results comparison:
  Current:   TOTAL: 317 / PASSED: 274 / FAILED: 37 / SKIPPED: 6
  Reference: TOTAL: 314 / PASSED: 266 / FAILED: 42 / SKIPPED: 6

Changes from main branch:
  TOTAL: +3
  PASSED: +8
  FAILED: -5

Test improvements (5):
  + gnu/execdir_path_dot
  + gnu/execdir_path_empty
  + gnu/execdir_path_relative
  + gnu/files0_from_ok
  + gnu/okdir_path_empty

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.

bug(xargs): when -a is /dev/full, it do not error out instead allocates memory indefinetively

3 participants