GeezerVisonThe World Wide Web, restored to 800 pixels of glory. |
| Home|About|Gaming cabinet|Archives|How it works|Starter sites |
GeezerVison is showing github.com — Fix /proc/<pid>/stat parsing when the command name contains whitespace by NopeCharon · Pull Request #2410 · pixie-io/pixie Conversation
Contributor
Summary: Fix parsing of The command name (
Since This PR adds a In the ASAN-only path of Relevant Issues: Fixes #678 Type of change: /kind bug Test Plan: Added parameterized tests to Changelog Message:
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions…pace
The command name (comm) field of /proc/<pid>/stat is surrounded by
parentheses and can contain any character, including runs of spaces,
newlines and parentheses. ProcParser::ParseProcPIDStat() and
GetPIDStartTimeTicks() handled this by splitting the first line of the
file on spaces with absl::SkipWhitespace() and then shifting every field
index by the number of spaces in the name. That breaks in several ways:
- Consecutive spaces in the name are collapsed by SkipWhitespace() but
are each counted in the offset, so later fields are read from the
wrong column (or out of bounds of the expected field count).
- Only the first line is read, so a newline in the name truncates the
record and the field-count check fails.
- The field-count check does not account for spaces in the name.
GetPIDStartTimeTicks() is used to build UPIDs, so a process with such a
name could get a wrong start time or be skipped altogether.
Replace the offset arithmetic with a SplitProcPIDStat() helper that
reads the whole file, takes the command name as everything between the
first '(' and the last ')' (no later field can contain a ')'), and splits
only the remainder on whitespace. The returned fields line up with the
existing kProcStat*Field constants, so callers index them directly.
In the ASAN-only code path of GetPIDStartTimeTicks(), read the file with
fread() instead of fgets() so that names containing newlines are
handled there too, trim the buffer to the bytes actually read, and close
the FILE handle, which was previously leaked.
Add parameterized tests covering names with consecutive, leading and
trailing spaces, newlines, unbalanced parentheses and an empty name for
both ParseProcPIDStat() and GetPIDStartTimeTicks().
Fixes pixie-io#678
Signed-off-by: NopeCharon <[email protected]>
NopeCharon
requested a deployment
to
pr-actions-approval
This branch is waiting to be deployed1 waiting deployment
pr-actions-approval
—
fc3f5a5c
Waiting
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
ReviewersNo reviewsUh oh!
There was an error while loading. Please reload this page. AssigneesNo one assignedLabels
None yet
ProjectsMilestoneNo milestoneDevelopmentSuccessfully merging this pull request may close these issues. /proc/PID/stat mishandles whitespaceUh oh!
There was an error while loading. Please reload this page. Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later. |
| You are visitor 01402520 — GeezerVison does not store the pages you look up. Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required. |