Repository: prometheus/procfs
Pull request #441 (2022-04-13 23:21:27)

Prevent panic if status line is empty in mdstat
This change fixes a potential source of panic when the status line in
mdstat is empty and the code is trying to reference the first item in a
zero-sized slice.

@discordianfish @pgier

Signed-off-by: Artur Makutunowicz <artur@makutunowicz.net>

discordianfish: Thanks, looks reasonable! You still need to sign-off your commit though. Ignore the circleci issues, we'll fix them.

And curious, where did you see mdstat having an empty line?
amakutunowicz: Looks like my public email setting was incorrect - can we somehow rerun the tests here? It should pass now.

I can't really say what exactly happened (I didn't log the mdstat contents), but sporadically I was able to see panics caused by that particular line.
SuperQ: DCO complains with this:

> Commit sha: [ed8f4a3](https://github.com/prometheus/procfs/pull/441/commits/ed8f4a367ab927a385b2d88b573f75cdf799b715), Author: Artur Makutunowicz, Committer: Artur Makutunowicz; Expected "Artur Makutunowicz [arma@linkedin.com](mailto:arma@linkedin.com)", but got "Artur Makutunowicz [artur@makutunowicz.net](mailto:artur@makutunowicz.net)".
SuperQ: You might need to edit the email in your git commit and force push.
pgier: You might need to rebase to pick up the lint fixes
amakutunowicz: Hello, is there anything else I can do to make this change merged and release a new version, so it can be picked up by Go modules?
SuperQ: If you could rebase your fork and branch, that should fix up the tests.

```
git checkout master
git remote add upstream https://github.com/prometheus/procfs.git
git fetch --all
git fetch upstream HEAD
git rebase FETCH_HEAD
git push
git checkout statusline-empty
git rebase -i origin/master
git push -f
```

amakutunowicz: @SuperQ Thanks, that seemed to work!

Patch:
diff --git a/mdstat.go b/mdstat.go
index f0b9e5f..2c7900f 100644
--- a/mdstat.go
+++ b/mdstat.go
@@ -166,8 +166,12 @@ func parseMDStat(mdStatData []byte) ([]MDStat, error) {
 }
 
 func evalStatusLine(deviceLine, statusLine string) (active, total, down, size int64, err error) {
+	statusFields := strings.Fields(statusLine)
+	if len(statusFields) < 1 {
+		return 0, 0, 0, 0, fmt.Errorf("unexpected statusLine %q", statusLine)
+	}
 
-	sizeStr := strings.Fields(statusLine)[0]
+	sizeStr := statusFields[0]
 	size, err = strconv.ParseInt(sizeStr, 10, 64)
 	if err != nil {
 		return 0, 0, 0, 0, fmt.Errorf("unexpected statusLine %q: %w", statusLine, err)


Test patch:
diff --git a/mdstat_test.go b/mdstat_test.go
index d572cb3..871e95a 100644
--- a/mdstat_test.go
+++ b/mdstat_test.go
@@ -56,16 +56,23 @@ func TestFS_MDStat(t *testing.T) {
 }
 
 func TestInvalidMdstat(t *testing.T) {
-	invalidMount := []byte(`
+	invalidMount := [][]byte{[]byte(`
 Personalities : [invalid]
 md3 : invalid
       314159265 blocks 64k chunks
 
 unused devices: <none>
-`)
+`),
+		[]byte(`
+md12 : active raid0 sdc2[0] sdd2[1]
 
-	_, err := parseMDStat(invalidMount)
-	if err == nil {
-		t.Fatalf("parsing of invalid reference file did not find any errors")
+      3886394368 blocks super 1.2 512k chunks
+`)}
+
+	for _, invalid := range invalidMount {
+		_, err := parseMDStat(invalid)
+		if err == nil {
+			t.Fatalf("parsing of invalid reference file did not find any errors")
+		}
 	}
 }