Tolerate mountinfo lines with an empty mount source #7044
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| //go:build !windows | ||
|
|
||
| package containerinfo | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "os" | ||
| "strings" | ||
| ) | ||
|
|
||
| // mountInfo holds the subset of /proc/<pid>/mountinfo fields that the | ||
| // container info extractor consumes. The full mountinfo line format is | ||
| // documented in proc(5). | ||
| type mountInfo struct { | ||
| // Root is the pathname of the directory in the filesystem which forms the | ||
| // root of this mount (field 4). | ||
| Root string | ||
| // FsType is the filesystem type (the first field after the "-" separator). | ||
| FsType string | ||
| } | ||
|
|
||
| // parseMountInfo parses /proc/<pid>/mountinfo. | ||
| // | ||
| // It exists because k8s.io/mount-utils ParseMountInfo splits each line with | ||
| // strings.Fields, which collapses runs of whitespace and therefore drops an | ||
| // empty mount source field. Per proc(5) the mount source (the field after the | ||
| // filesystem type) may be empty (for example a tmpfs mount has no source), and | ||
| // the kernel escapes any real whitespace in a path as octal (\040). A line | ||
| // like: | ||
| // | ||
| // 119 206 0:68 / /local rw,relatime - tmpfs rw,size=8192k | ||
| // | ||
| // is therefore valid: the double space between "tmpfs" and "rw,size=8192k" | ||
| // unambiguously means an empty source. The upstream parser counts that as 9 | ||
| // fields (it expects at least 10) and rejects the entire file, which made the | ||
| // docker and k8s workload attestors fail attestation outright. This parser is | ||
| // position-aware around the "-" separator so an empty source is tolerated. | ||
| func parseMountInfo(filename string) ([]mountInfo, error) { | ||
| 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) | ||
| } | ||
| return infos, nil | ||
| } | ||
|
|
||
| // parseMountInfoLine parses a single mountinfo line. The format (see proc(5)) | ||
| // is, in order: | ||
| // | ||
| // (1) mount ID (2) parent ID (3) major:minor (4) root (5) mount point | ||
| // (6) mount options (7..) zero or more optional fields (8) a "-" separator | ||
| // (9) filesystem type (10) mount source (11) super options | ||
| // | ||
| // Only the fields the extractor needs (root and filesystem type) are returned. | ||
| // Fields up to the "-" separator are whitespace-separated and never empty, so | ||
| // strings.Fields is safe there. The three fields after the separator are taken | ||
| // by position so an empty source (field 10) is preserved rather than collapsed. | ||
| func parseMountInfoLine(line string) (mountInfo, error) { | ||
| // Split the line into the part before the "-" separator and the part after | ||
| // it. The separator is a standalone "-" token, so it is surrounded by | ||
| // 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, " - ") | ||
|
Member
There was a problem hiding this comment. Looking at what
Author
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. |
||
| if sep < 0 { | ||
| return mountInfo{}, fmt.Errorf("missing separator in mountinfo line: %s", line) | ||
| } | ||
|
|
||
| before := strings.Fields(line[:sep]) | ||
| // Fields before the separator: mount ID, parent ID, major:minor, root, | ||
| // mount point, mount options, then zero or more optional fields. The root | ||
| // is field index 3 (zero-based). | ||
| if len(before) < 6 { | ||
| return mountInfo{}, fmt.Errorf("expected at least 6 fields before separator in mountinfo line: %s", line) | ||
| } | ||
|
|
||
| // 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) | ||
| } | ||
|
|
||
|
|
||
| return mountInfo{ | ||
| Root: before[3], | ||
| FsType: after[0], | ||
| }, nil | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| //go:build !windows | ||
|
|
||
| package containerinfo | ||
|
|
||
| import ( | ||
| "os" | ||
| "path/filepath" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestParseMountInfoLine(t *testing.T) { | ||
| for _, tt := range []struct { | ||
| name string | ||
| line string | ||
| wantRoot string | ||
| wantType string | ||
| wantErr string | ||
| }{ | ||
| { | ||
| name: "normal cgroup2 line", | ||
| line: "1543 1542 0:32 /some/root /sys/fs/cgroup ro,nosuid,nodev,noexec,relatime - cgroup2 cgroup rw,nsdelegate", | ||
| wantRoot: "/some/root", | ||
| wantType: "cgroup2", | ||
| }, | ||
| { | ||
| name: "optional fields before separator", | ||
| line: "573 572 0:33 /docker/abc /sys/fs/cgroup/systemd ro,nosuid,nodev,noexec,relatime master:11 - cgroup cgroup rw,name=systemd", | ||
| wantRoot: "/docker/abc", | ||
| wantType: "cgroup", | ||
| }, | ||
| { | ||
| // Regression for #7036: a tmpfs mount has no source, so the field | ||
| // after the filesystem type is empty. strings.Fields would collapse | ||
| // it and the upstream parser rejected the whole file. | ||
| name: "tmpfs with empty source", | ||
| line: "119 206 0:68 / /local rw,relatime - tmpfs rw,size=8192k", | ||
| wantRoot: "/", | ||
| wantType: "tmpfs", | ||
| }, | ||
| { | ||
| name: "missing separator", | ||
| line: "1543 1542 0:32 /some/root /sys/fs/cgroup ro,nosuid,nodev,noexec,relatime cgroup2 cgroup rw", | ||
| wantErr: "missing separator", | ||
| }, | ||
| { | ||
| name: "too few fields before separator", | ||
| line: "1543 1542 0:32 - cgroup2 cgroup rw", | ||
| wantErr: "expected at least 6 fields before separator", | ||
| }, | ||
| { | ||
| name: "missing filesystem type after separator", | ||
| line: "1543 1542 0:32 /some/root /sys/fs/cgroup rw - ", | ||
| wantErr: "missing filesystem type", | ||
| }, | ||
| } { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| info, err := parseMountInfoLine(tt.line) | ||
| if tt.wantErr != "" { | ||
| assert.ErrorContains(t, err, tt.wantErr) | ||
| return | ||
| } | ||
| require.NoError(t, err) | ||
| assert.Equal(t, tt.wantRoot, info.Root) | ||
| assert.Equal(t, tt.wantType, info.FsType) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestParseMountInfo(t *testing.T) { | ||
| dir := t.TempDir() | ||
| path := filepath.Join(dir, "mountinfo") | ||
| content := "" + | ||
| "2356 2355 0:30 /../containerid /sys/fs/cgroup ro,nosuid,nodev,noexec,relatime - cgroup2 cgroup rw\n" + | ||
| "119 206 0:68 / /local rw,relatime - tmpfs rw,size=8192k\n" | ||
| require.NoError(t, os.WriteFile(path, []byte(content), 0o600)) | ||
|
|
||
| infos, err := parseMountInfo(path) | ||
| require.NoError(t, err) | ||
| require.Len(t, infos, 2) | ||
|
|
||
| assert.Equal(t, "/../containerid", infos[0].Root) | ||
| assert.Equal(t, "cgroup2", infos[0].FsType) | ||
|
|
||
| // The tmpfs line with an empty source must still parse. | ||
| assert.Equal(t, "/", infos[1].Root) | ||
| assert.Equal(t, "tmpfs", infos[1].FsType) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| 2356 2355 0:30 /../0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef /sys/fs/cgroup ro,nosuid,nodev,noexec,relatime - cgroup2 cgroup rw | ||
| 119 206 0:68 / /local rw,relatime - tmpfs rw,size=8192k |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems useful to do and could simplify things a bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in the same commit (4af7f2f). Swapped ReadFile + Split for bufio.Scanner, so it streams line by line now.