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 1/2] 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)
+	}
+}

From 3b53114ad1a853e568de42d8e71e83ef89cdec54 Mon Sep 17 00:00:00 2001
From: Julio Montes <julio.montes@intel.com>
Date: Fri, 20 Mar 2020 19:28:23 +0000
Subject: [PATCH 2/2] virtcontainers: improve algorithm to check Large bar
 devices

Instead of iterate in a loop dividing bytes by 1024, use right shift
to convert Bytes to GBytes and check if that number is greater than 4G

Signed-off-by: Julio Montes <julio.montes@intel.com>
---
 virtcontainers/device/manager/utils.go | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/virtcontainers/device/manager/utils.go b/virtcontainers/device/manager/utils.go
index 5423576f44..7a437f958b 100644
--- a/virtcontainers/device/manager/utils.go
+++ b/virtcontainers/device/manager/utils.go
@@ -102,7 +102,6 @@ func isLargeBarSpace(resourcePath string) (bool, error) {
 	// Refer:
 	// resource format: https://github.com/torvalds/linux/blob/63623fd44972d1ed2bfb6e0fb631dfcf547fd1e7/drivers/pci/pci-sysfs.c#L145
 	// calculate size : https://github.com/pciutils/pciutils/blob/61ecc14a327de030336f1ff3fea9c7e7e55a90ca/lspci.c#L388
-	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
@@ -119,25 +118,18 @@ func isLargeBarSpace(resourcePath string) (bool, error) {
 			}).Debug("start is greater than end")
 			continue
 		}
-		size := end - start + 1
-		sIdx := 0
-		for i := range suffix {
-			if size/1024 < 1 {
-				break
-			}
-			size /= 1024
-			sIdx = i + 1
-		}
+		// Use right shift to convert Bytes to GBytes
+		// This is equivalent to ((end - start + 1) / 1024 / 1024 / 1024)
+		gbSize := (end - start + 1) >> 30
 		deviceLogger().WithFields(logrus.Fields{
 			"resource": resourcePath,
 			"region":   rIdx,
 			"start":    cols[0],
 			"end":      cols[1],
-			"size":     size,
-			"suffix":   suffix[sIdx],
+			"gb-size":  gbSize,
 		}).Debug("Check large bar space device")
 		//size is large than 4G
-		if (sIdx == 3 && size > 4) || sIdx > 3 {
+		if gbSize > 4 {
 			return true, nil
 		}
 	}