From 7aff54665550355957ee3418430792fe46007f7a Mon Sep 17 00:00:00 2001
From: Julio Montes <julio.montes@intel.com>
Date: Fri, 20 Mar 2020 17:27:39 +0000
Subject: [PATCH] virtcontainers: check PCI resource format before using it

Make sure the number of columns in the PCI resource file is greater
or equal to 2, since the first two columns are used to calculate
the PCI bar space.
Add unit test for `isLargeBarSpace()`.

fixes #2542

Signed-off-by: Julio Montes <julio.montes@intel.com>
---
 virtcontainers/device/manager/utils.go      | 12 ++++++
 virtcontainers/device/manager/utils_test.go | 45 +++++++++++++++++++++
 2 files changed, 57 insertions(+)

diff --git a/virtcontainers/device/manager/utils.go b/virtcontainers/device/manager/utils.go
index 5f703b1b13..5423576f44 100644
--- a/virtcontainers/device/manager/utils.go
+++ b/virtcontainers/device/manager/utils.go
@@ -105,8 +105,20 @@ func isLargeBarSpace(resourcePath string) (bool, error) {
 	suffix := []string{"", "K", "M", "G", "T"}
 	for rIdx, line := range strings.Split(string(buf), "\n") {
 		cols := strings.Fields(line)
+		// start and end columns are required to calculate the size
+		if len(cols) < 2 {
+			deviceLogger().WithField("resource-line", line).Debug("not enough columns to calculate PCI size")
+			continue
+		}
 		start, _ := strconv.ParseUint(cols[0], 0, 64)
 		end, _ := strconv.ParseUint(cols[1], 0, 64)
+		if start > end {
+			deviceLogger().WithFields(logrus.Fields{
+				"start": start,
+				"end":   end,
+			}).Debug("start is greater than end")
+			continue
+		}
 		size := end - start + 1
 		sIdx := 0
 		for i := range suffix {
diff --git a/virtcontainers/device/manager/utils_test.go b/virtcontainers/device/manager/utils_test.go
index 33c650b622..5c1ab643e3 100644
--- a/virtcontainers/device/manager/utils_test.go
+++ b/virtcontainers/device/manager/utils_test.go
@@ -7,6 +7,8 @@
 package manager
 
 import (
+	"io/ioutil"
+	"os"
 	"testing"
 
 	"github.com/kata-containers/runtime/virtcontainers/device/config"
@@ -89,3 +91,46 @@ func TestIsVhostUserSCSI(t *testing.T) {
 		assert.Equal(t, d.expected, isVhostUserSCSI)
 	}
 }
+
+func TestIsLargeBarSpace(t *testing.T) {
+	assert := assert.New(t)
+
+	// File not exist
+	bs, err := isLargeBarSpace("/abc/xyz/123/rgb")
+	assert.Error(err)
+	assert.False(bs)
+
+	f, err := ioutil.TempFile("", "pci")
+	assert.NoError(err)
+	defer f.Close()
+	defer os.RemoveAll(f.Name())
+
+	type testData struct {
+		resourceInfo string
+		error        bool
+		result       bool
+	}
+
+	for _, d := range []testData{
+		{"", false, false},
+		{"\t\n\t  ", false, false},
+		{"abc zyx", false, false},
+		{"abc zyx rgb", false, false},
+		{"abc\t       zyx     \trgb", false, false},
+		{"0x00015\n0x0013", false, false},
+		{"0x00000000c6000000 0x00000000c6ffffff 0x0000000000040200", false, false},
+		{"0x0000383bffffffff 0x0000383800000000", false, false}, // start greater than end
+		{"0x0000383800000000 0x0000383bffffffff", false, true},
+		{"0x0000383800000000 0x0000383bffffffff 0x000000000014220c", false, true},
+	} {
+		f.WriteAt([]byte(d.resourceInfo), 0)
+		bs, err = isLargeBarSpace(f.Name())
+		assert.NoError(f.Truncate(0))
+		if d.error {
+			assert.Error(err, d.resourceInfo)
+		} else {
+			assert.NoError(err, d.resourceInfo)
+		}
+		assert.Equal(d.result, bs, d.resourceInfo)
+	}
+}