From 7f008f4d5642e59442cd9fea5dea616317412ff5 Mon Sep 17 00:00:00 2001
From: Alex K <8418476+fearful-symmetry@users.noreply.github.com>
Date: Mon, 4 Mar 2024 09:08:03 -0800
Subject: [PATCH] Check for nil hostfs option while setting up cgroups (#131)

## 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`
---
 metric/system/cgroup/reader.go      | 14 +++++---------
 metric/system/cgroup/reader_test.go |  9 +++++++++
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/metric/system/cgroup/reader.go b/metric/system/cgroup/reader.go
index 33e84f4ace..af83d849dc 100644
--- a/metric/system/cgroup/reader.go
+++ b/metric/system/cgroup/reader.go
@@ -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
@@ -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.
diff --git a/metric/system/cgroup/reader_test.go b/metric/system/cgroup/reader_test.go
index 09e32832a5..196cb9d59e 100644
--- a/metric/system/cgroup/reader_test.go
+++ b/metric/system/cgroup/reader_test.go
@@ -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)