Tolerate mountinfo lines with an empty mount source#7044
Conversation
The docker workload attestor (and k8s, which shares the code) parses
/proc/<pid>/mountinfo via k8s.io/mount-utils ParseMountInfo, which splits
each line with strings.Fields and requires at least 10 fields. Per proc(5)
the mount source may be empty (a tmpfs mount has no source), which renders
as an empty field between the filesystem type and the super options. The
kernel escapes any real whitespace in a path as octal (\040), so the
double space in a line like:
119 206 0:68 / /local rw,relatime - tmpfs rw,size=8192k
unambiguously means an empty source. strings.Fields collapses that empty
field, so the line counts as 9 fields and ParseMountInfo rejects the whole
file with "wrong number of fields". The attestor then extracts no
container ID and no SVID is issued.
Replace the upstream call with a small SPIRE-side mountinfo parser that is
position-aware around the "-" separator, so the trailing fstype/source/
super-options triple is read by position and an empty source is tolerated.
Only the root and filesystem type are consumed by the extractor. This also
drops the k8s.io/mount-utils dependency (and its transitive moby/sys/
mountinfo) since it was the only user.
Adds a regression fixture with the empty-source tmpfs line plus unit tests
for the parser.
Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
arpitjain099
requested review from
MarcosDY,
amartinezfayo,
evan2645,
rturner3 and
sorindumitru
as code owners
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an internal /proc/<pid>/mountinfo parser to tolerate tmpfs mounts with an empty mount source field, preventing container ID extraction from failing due to upstream parsing behavior.
Changes:
- Introduces
parseMountInfo/parseMountInfoLineto parse mountinfo without rejecting valid tmpfs lines with an empty source. - Switches container ID extraction to use the new parser instead of
k8s.io/mount-utils. - Adds regression tests and testdata covering the empty-source tmpfs case.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/common/containerinfo/mountinfo.go | Implements a mountinfo parser tolerant of empty mount source fields. |
| pkg/common/containerinfo/mountinfo_test.go | Adds unit tests for mountinfo line/file parsing, including the regression case. |
| pkg/common/containerinfo/extract.go | Uses the new mountinfo parser during container ID extraction. |
| pkg/common/containerinfo/extract_test.go | Adds an end-to-end regression test ensuring extraction succeeds with empty-source tmpfs. |
| pkg/common/containerinfo/testdata/docker/tmpfs-empty-source/proc/123/mountinfo | Adds mountinfo fixture containing an empty-source tmpfs mount. |
| go.mod / go.sum | Removes now-unneeded dependencies (k8s.io/mount-utils, github.com/moby/sys/mountinfo). |
| // After the separator there are exactly three positional fields: | ||
| // filesystem type, mount source, and super options. The source may be | ||
| // empty, so parse by splitting into at most three fields and keeping the | ||
| // first (filesystem type). Trailing whitespace from an empty source does | ||
| // not affect the filesystem type, which is the first non-empty token. | ||
| after := strings.Fields(line[sep+len(" - "):]) | ||
| if len(after) < 1 { | ||
| return mountInfo{}, fmt.Errorf("missing filesystem type in mountinfo line: %s", line) | ||
| } |
| content, err := os.ReadFile(filename) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| var infos []mountInfo | ||
| for _, line := range strings.Split(string(content), "\n") { | ||
| if line == "" { | ||
| continue | ||
| } | ||
| info, err := parseMountInfoLine(line) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| infos = append(infos, info) | ||
| } |
There was a problem hiding this comment.
This seems useful to do and could simplify things a bit.
There was a problem hiding this comment.
Done in the same commit (4af7f2f). Swapped ReadFile + Split for bufio.Scanner, so it streams line by line now.
|
Thank you! |
|
Thanks for opening this PR @arpitjain099
Could you share a source for this? My Could you share some details about the system where you encountered this or a way to reproduce (e.g. can I just start a docker container with some options)? |
|
Thanks @sorindumitru! This is specifically the tmpfs case. You're right that The concrete line is in #7036 (filed by @amartinezfayo): Note the double space between To reproduce with Docker: You should see the tmpfs line with the empty source. A Kubernetes pod with an |
|
@arpitjain099 this is what I get from that: This is with a recent kernel version, though 7.0. Are you running this on linux, and which distro/kernel version, or some other operating system? |
Was able to reproduce by calling |
|
Thanks for digging in @sorindumitru, and good that the To be upfront about the environment question: I didn't hit this on my own machine. The concrete line came from @amartinezfayo's report in #7036 (a real workload, As you found, the trigger is the mount source being literally empty (a |
|
Validated that this change doesn't trigger the parser issue. |
| content, err := os.ReadFile(filename) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| var infos []mountInfo | ||
| for _, line := range strings.Split(string(content), "\n") { | ||
| if line == "" { | ||
| continue | ||
| } | ||
| info, err := parseMountInfoLine(line) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| infos = append(infos, info) | ||
| } |
There was a problem hiding this comment.
This seems useful to do and could simplify things a bit.
| // spaces. Splitting on " - " keeps it unambiguous: optional-field tags | ||
| // before the separator never equal a bare "-", and the post-separator | ||
| // fields are matched by position below. | ||
| sep := strings.Index(line, " - ") |
There was a problem hiding this comment.
Looking at what strings offers, I think strings.Split(line, " ") should do what we want here and maybe it would simplify things a bit. What do you think?
There was a problem hiding this comment.
Good catch. Switched to strings.Split(line, " ") so empty fields survive as empty strings in the slice. This lets us locate the separator and all post-separator fields by index directly, no more separate Index + Fields split. Pushed in 4af7f2f.
Switch parseMountInfoLine from the Index+Fields approach to strings.Split(line, " "), which preserves empty fields between consecutive spaces. This makes the empty-source handling more direct: the separator and all fields are located by index in a single slice. Also switch parseMountInfo to stream lines with bufio.Scanner instead of reading the whole file into memory first. Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
| sepIdx := -1 | ||
| for i, f := range fields { | ||
| if f == "-" { | ||
| sepIdx = i | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
I think this should work here, but might need an extra import:
| sepIdx := -1 | |
| for i, f := range fields { | |
| if f == "-" { | |
| sepIdx = i | |
| break | |
| } | |
| } | |
| sepIdx := slices.Index(fields, "-") |
|
Hi @arpitjain099, do you think you'll have time to address that small comment I left and resolve the conflicts? If you won't I can try to push the changes over the branch, but we'd want to include this in the next release. |
Pull Request check list
Affected functionality
The docker and k8s workload attestors, which share
pkg/common/containerinfoto read/proc/<pid>/mountinfoand recover the container ID.Description of change
Implements the SPIRE-side approach confirmed on #7036. The attestor parses mountinfo via
k8s.io/mount-utilsParseMountInfo, which splits each line withstrings.Fieldsand requires at least 10 fields. Per proc(5) the mount source may be empty (a tmpfs mount has no source), rendering as an empty field between the filesystem type and the super options. The kernel escapes any real whitespace in a path as octal (\040), so the double space in a line like:unambiguously means an empty source.
strings.Fieldscollapses that empty field, so the line counts as 9 fields andParseMountInforejects the whole file with "wrong number of fields"; the attestor then extracts no container ID and no SVID is issued.This replaces the upstream call with a small SPIRE-side mountinfo parser that is position-aware around the
-separator, so the trailing fstype/source/super-options triple is read by position and an empty source is tolerated. Only the mount root and filesystem type are consumed by the extractor. Since this was the only user ofk8s.io/mount-utils, the change also drops that dependency (and its transitivemoby/sys/mountinfo).A regression fixture with the empty-source tmpfs line and unit tests for the parser are included.
Which issue this PR fixes
fixes #7036