merge branch 'pr-2529'

Aleksa Sarai (1):
  devices: correctly check device types

LGTMs: @AkihiroSuda @mrunalp
Closes #2529

libcontainer/devices/devices.go

@@ -37,12 +37,12 @@ func DeviceFromPath(path, permissions string) (*configs.Device, error) {
 		major     = unix.Major(devNumber)
 		minor     = unix.Minor(devNumber)
 	)
-	switch {
-	case mode&unix.S_IFBLK == unix.S_IFBLK:
+	switch mode & unix.S_IFMT {
+	case unix.S_IFBLK:
 		devType = configs.BlockDevice
-	case mode&unix.S_IFCHR == unix.S_IFCHR:
+	case unix.S_IFCHR:
 		devType = configs.CharDevice
-	case mode&unix.S_IFIFO == unix.S_IFIFO:
+	case unix.S_IFIFO:
 		devType = configs.FifoDevice
 	default:
 		return nil, ErrNotADevice
@@ -104,6 +104,9 @@ func GetDevices(path string) ([]*configs.Device, error) {
 			}
 			return nil, err
 		}
+		if device.Type == configs.FifoDevice {
+			continue
+		}
 		out = append(out, device)
 	}
 	return out, nil

libcontainer/devices/devices_test.go

@@ -2,12 +2,19 @@ package devices
 
 import (
 	"errors"
+	"io/ioutil"
 	"os"
 	"testing"
 
+	"github.com/opencontainers/runc/libcontainer/configs"
 	"golang.org/x/sys/unix"
 )
 
+func cleanupTest() {
+	unixLstat = unix.Lstat
+	ioutilReadDir = ioutil.ReadDir
+}
+
 func TestDeviceFromPathLstatFailure(t *testing.T) {
 	testError := errors.New("test error")
 
@@ -15,6 +22,7 @@ func TestDeviceFromPathLstatFailure(t *testing.T) {
 	unixLstat = func(path string, stat *unix.Stat_t) error {
 		return testError
 	}
+	defer cleanupTest()
 
 	_, err := DeviceFromPath("", "")
 	if err != testError {
@@ -29,6 +37,7 @@ func TestHostDevicesIoutilReadDirFailure(t *testing.T) {
 	ioutilReadDir = func(dirname string) ([]os.FileInfo, error) {
 		return nil, testError
 	}
+	defer cleanupTest()
 
 	_, err := HostDevices()
 	if err != testError {
@@ -55,9 +64,41 @@ func TestHostDevicesIoutilReadDirDeepFailure(t *testing.T) {
 
 		return []os.FileInfo{fi}, nil
 	}
+	defer cleanupTest()
 
 	_, err := HostDevices()
 	if err != testError {
 		t.Fatalf("Unexpected error %v, expected %v", err, testError)
 	}
 }
+
+func TestHostDevicesAllValid(t *testing.T) {
+	devices, err := HostDevices()
+	if err != nil {
+		t.Fatalf("failed to get host devices: %v", err)
+	}
+
+	for _, device := range devices {
+		// Devices can't have major number 0.
+		if device.Major == 0 {
+			t.Errorf("device entry %+v has zero major number", device)
+		}
+		// Devices should only have file modes that correspond to their type.
+		var expectedType os.FileMode
+		switch device.Type {
+		case configs.BlockDevice:
+			expectedType = unix.S_IFBLK
+		case configs.CharDevice:
+			expectedType = unix.S_IFCHR
+		case configs.FifoDevice:
+			t.Logf("fifo devices shouldn't show up from HostDevices")
+			fallthrough
+		default:
+			t.Errorf("device entry %+v has unexpected type %v", device, device.Type)
+		}
+		gotType := device.FileMode & unix.S_IFMT
+		if expectedType != gotType {
+			t.Errorf("device entry %+v has mismatched types (expected %#x, got %#x)", device, expectedType, gotType)
+		}
+	}
+}