iperf_parse_arguments() spuriously fails on 2nd+ call in the same process: stale i_errno from previous run's normal completion misread as new parse failure
- Dominant language
- C
- Stars
- 8.8k
- Forks
- 1.5k
- PR merge metrics
- No merged PRs in 30d
Description
### Summary
`iperf_parse_arguments()` can incorrectly return `-1` on any call after the
first one in a long-lived process (e.g. a mobile app embedding `libiperf`
and running successive tests), because several option cases check
`i_errno` for "did the conversion I just did fail?" without first resetting
it — and `i_errno` is never cleared to 0 anywhere after the library
initializes it once at load time.
This doesn't affect the `iperf3` CLI binary itself, since `main.c` only
calls `iperf_parse_arguments()` once per process. It affects any library
caller that constructs a new `struct iperf_test` and re-parses arguments
multiple times in one process — which is the normal, documented way to use
`libiperf` as an embedded library rather than spawning `iperf3` as a
subprocess.
### Root cause
`i_errno` is declared and zero-initialized once:
https://github.com/esnet/iperf/blob/3.21/src/iperf_error.c#L160
```c
int i_errno = 0;
It is never reset anywhere else in the library. unit_atoi() /
unit_atof() / unit_atof_rate() (src/units.c) only ever set
i_errno on their own failure — they never clear it on success:
https://github.com/esnet/iperf/blob/3.21/src/units.c#L185-L222
Several iperf_parse_arguments() option cases (src/iperf_api.c) call one
of those helpers and then check if (i_errno != 0) return -1; as their
only error signal — relying on i_errno having been 0 going into the
call:
┌──────────────────────────┬────────────────────────────────────────────────────────────────────────────────────┐
│ Option │ Line │
├──────────────────────────┼────────────────────────────────────────────────────────────────────────────────────┤
│ OPT_NUMSTREAMS │ iperf_api.c#L1338 (https://github.com/esnet/iperf/blob/3.21/src/iperf_api.c#L1338) │
├──────────────────────────┼────────────────────────────────────────────────────────────────────────────────────┤
│ -b / --bitrate │ iperf_api.c#L1359 (https://github.com/esnet/iperf/blob/3.21/src/iperf_api.c#L1359) │
├──────────────────────────┼────────────────────────────────────────────────────────────────────────────────────┤
│ OPT_SERVER_BITRATE_LIMIT │ iperf_api.c#L1378 (https://github.com/esnet/iperf/blob/3.21/src/iperf_api.c#L1378) │
├──────────────────────────┼────────────────────────────────────────────────────────────────────────────────────┤
│ -n │ iperf_api.c#L1394 (https://github.com/esnet/iperf/blob/3.21/src/iperf_api.c#L1394) │
├──────────────────────────┼────────────────────────────────────────────────────────────────────────────────────┤
│ -k │ iperf_api.c#L1401 (https://github.com/esnet/iperf/blob/3.21/src/iperf_api.c#L1401) │
├──────────────────────────┼────────────────────────────────────────────────────────────────────────────────────┤
│ -l │ iperf_api.c#L1408 (https://github.com/esnet/iperf/blob/3.21/src/iperf_api.c#L1408) │
├──────────────────────────┼────────────────────────────────────────────────────────────────────────────────────┤
│ -w / --window │ iperf_api.c#L1442 (https://github.com/esnet/iperf/blob/3.21/src/iperf_api.c#L1442) │
├──────────────────────────┼────────────────────────────────────────────────────────────────────────────────────┤
│ --fq-rate │ iperf_api.c#L1728 (https://github.com/esnet/iperf/blob/3.21/src/iperf_api.c#L1728) │
├──────────────────────────┼────────────────────────────────────────────────────────────────────────────────────┤
│ --pacing-timer │ iperf_api.c#L1780 (https://github.com/esnet/iperf/blob/3.21/src/iperf_api.c#L1780) │
├──────────────────────────┼────────────────────────────────────────────────────────────────────────────────────┤
│ --connect-timeout │ iperf_api.c#L1787 (https://github.com/esnet/iperf/blob/3.21/src/iperf_api.c#L1787) │
└──────────────────────────┴────────────────────────────────────────────────────────────────────────────────────┘
Separately, a normal, successful test run can itself leave i_errno set
to IESTREAMREAD (206) as an ordinary side effect of how the receive path
detects end-of-stream (observed directly via logging around
iperf_run_client() — i_errno was 206 immediately after a run that
completed and returned 0/success). That leftover value then persists into
the next call to iperf_parse_arguments() in the same process. If that
next call's argv includes any of the options above (e.g. --connect-timeout,
which is a very commonly-used option for any client), the
if (i_errno != 0) return -1; check fires immediately — before any real
parsing problem — misreporting a perfectly valid argument list as a parse
failure, with a stale/unrelated error message (whatever i_errno happened
to be, e.g. "unable to read from stream socket" from the previous run).
Note that --connect-timeout did not have this check at all in 3.19
(iperf-3.19 iperf_api.c#L1703-L1706 (https://github.com/esnet/iperf/blob/3.19/src/iperf_api.c#L1703-L1706)
— it just assigned the value with no i_errno check); the guard appears to
have been added later, which is presumably when this regression was
introduced for library callers that reuse a process across multiple runs.
Reproduction
Using libiperf directly (not the iperf3 CLI) in a single process:
1. iperf_new_test() + iperf_defaults() + iperf_parse_arguments() with
--client --port --connect-timeout 2000 ... and run a
normal, successful client test to completion (iperf_run_client()).
iperf_free_test().
2. Repeat step 1 again in the same process — iperf_new_test() a fresh
struct iperf_test, and call iperf_parse_arguments() with the same or
different valid arguments.
3. Step 2's iperf_parse_arguments() returns -1, even though the
arguments are valid — because i_errno was left non-zero by step 1's
normal completion, and the --connect-timeout case (or any of the other
options in the table above) treats that stale value as "the conversion I
just did failed."
Suggested fix
Reset i_errno = 0; at the start of iperf_parse_arguments(), before the
getopt_long() loop begins — mirroring the optind/optreset reset that
already exists at the end of the function for repeat-call support (#ifdef __APPLE__ block), which suggests re-entrant/repeated calls were already a
design consideration, just not fully realized for i_errno.
Environment
- iperf 3.21 (tag 3.21, commit d39cf41)
- Reproduced via a custom JNI bridge embedding libiperf in an
Android/Kotlin app (client-only build, --without-sctp --with-openssl=no)
running repeated client tests in one process — but the bug is in
iperf_parse_arguments() itself and isn't specific to that environment.
Contributor guide
Research direction
Start in src/iperf_api.c at iperf_parse_arguments(), then read i_errno in src/iperf_error.c and the conversion helpers in src/units.c. Reproduce repeated library parses after a successful run using the documented options, and verify valid arguments no longer fail while genuine conversion errors still do.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100