Skip to content

Commit

Permalink
Check for nil hostfs option while setting up cgroups (#131)
Browse files Browse the repository at this point in the history
## What does this PR do?

This fixes a small bug where if `opts.RootfsMountpoint` isn't set when
we call `NewReaderOptions()`, it can cause a panic.

## Why is it important?

We don't want panics.

## Checklist

- [x] My code follows the style guidelines of this project
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] I have added tests that prove my fix is effective or that my
feature works
- [ ] I have added an entry in `CHANGELOG.md`
  • Loading branch information
fearful-symmetry authored Mar 4, 2024
1 parent 5c1fcf2 commit 7f008f4
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 9 deletions.
14 changes: 5 additions & 9 deletions metric/system/cgroup/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,6 @@ const (
memoryStat = "memory"
)

//nolint: deadcode,structcheck,unused // needed by other platforms
type mount struct {
subsystem string // Subsystem name (e.g. cpuacct).
mountpoint string // Mountpoint of the subsystem (e.g. /cgroup/cpuacct).
path string // Relative path to the cgroup (e.g. /docker/<id>).
id string // ID of the cgroup.
fullPath string // Absolute path to the cgroup. It's the mountpoint joined with the path.
}

// pathListWithTime combines PathList with a timestamp.
type pathListWithTime struct {
added time.Time
Expand Down Expand Up @@ -133,6 +124,11 @@ func NewReader(rootfsMountpoint resolve.Resolver, ignoreRootCgroups bool) (*Read

// NewReaderOptions creates and returns a new Reader with the given options.
func NewReaderOptions(opts ReaderOptions) (*Reader, error) {
// we don't want a nil pointer deref if someone forgets to set this
if opts.RootfsMountpoint == nil {
opts.RootfsMountpoint = resolve.NewTestResolver("/")
}

// Determine what subsystems are supported by the kernel.
subsystems, err := SupportedSubsystems(opts.RootfsMountpoint)
// We can return a not-quite-an-error ErrCgroupsMissing here, so return the bare error.
Expand Down
9 changes: 9 additions & 0 deletions metric/system/cgroup/reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,15 @@ const (
idv2 = "docker-1c8fa019edd4b9d4b2856f4932c55929c5c118c808ed5faee9a135ca6e84b039.scope"
)

func TestReaderOptsWithoutResolve(t *testing.T) {
reader, err := NewReaderOptions(ReaderOptions{})
require.NoError(t, err)

// actual value doesn't matter, the point is that we don't set RootfsMountpoint and we __don't__ get a nil pointer deref panic.
_, err = reader.CgroupsVersion(345)
require.Error(t, err)
}

func TestV1EventDifferentPaths(t *testing.T) {
pid := 3757
reader, err := NewReader(resolve.NewTestResolver("testdata/ubuntu1804"), true)
Expand Down

0 comments on commit 7f008f4

Please sign in to comment.