Skip to content

Commit

Permalink
mountinfo: run filter after all fields are parsed
Browse files Browse the repository at this point in the history
Previously, the filter would be run before all of the fields were parsed
(this behaviour also was not documented) -- this resulted in users of
the function accidentally assuming that fields like fsinfo.Root would
actually be filled correctly. It seems that the performance overhead of
parsing a few extra fields is not exorbitant, and optimising this just
leads to incorrect user code.

For a concrete example, this optimisation actually made this runc
change[1] regress a security hardening feature because it relied on
fsinfo.Root being filled correctly.

[1]: opencontainers/runc#2647

Signed-off-by: Aleksa Sarai <[email protected]>
  • Loading branch information
cyphar committed Oct 21, 2020
1 parent 99cfd57 commit 41fc4b7
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 16 deletions.
5 changes: 2 additions & 3 deletions mountinfo/mountinfo_filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ import "strings"
// used to filter out mountinfo entries we're not interested in,
// and/or stop further processing if we found what we wanted.
//
// It takes a pointer to the Info struct (not fully populated,
// currently only Mountpoint, FSType, Source, and (on Linux)
// VFSOptions are filled in), and returns two booleans:
// It takes a pointer to the Info struct (fully populated with all available
// fields on the GOOS platform), and returns two booleans:
//
// skip: true if the entry should be skipped;
//
Expand Down
22 changes: 9 additions & 13 deletions mountinfo/mountinfo_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ func GetMountsFromReader(r io.Reader, filter FilterFunc) ([]*Info, error) {

p := &Info{}

// Fill in the fields that a filter might check
p.Mountpoint, err = unescape(fields[4])
if err != nil {
return nil, fmt.Errorf("Parsing '%s' failed: mount point: %w", fields[4], err)
Expand All @@ -87,18 +86,6 @@ func GetMountsFromReader(r io.Reader, filter FilterFunc) ([]*Info, error) {
}
p.VFSOptions = fields[sepIdx+3]

// Run a filter early so we can skip parsing/adding entries
// the caller is not interested in
var skip, stop bool
if filter != nil {
skip, stop = filter(p)
if skip {
continue
}
}

// Fill in the rest of the fields

// ignore any numbers parsing errors, as there should not be any
p.ID, _ = strconv.Atoi(fields[0])
p.Parent, _ = strconv.Atoi(fields[1])
Expand Down Expand Up @@ -126,6 +113,15 @@ func GetMountsFromReader(r io.Reader, filter FilterFunc) ([]*Info, error) {
p.Optional = strings.Join(fields[6:sepIdx-1], " ")
}

// Run the filter after parsing all of the fields.
var skip, stop bool
if filter != nil {
skip, stop = filter(p)
if skip {
continue
}
}

out = append(out, p)
if stop {
break
Expand Down

0 comments on commit 41fc4b7

Please sign in to comment.