esnet / esnet/iperf

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

Open Beginner friendly
#2,063 1 comment 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.