Ensure that filemode is set for device nodes

This change ensures that the filemode for a CDI device is
properly set from the host device node when adding devices
to the CDI spec.

Without this the device node permissions are 000 in the container
under certain conditions (such as Kata).

Signed-off-by: Evan Lezar <[email protected]>

Author: elezar

pkg/cdi/container-edits_test.go

@@ -17,6 +17,7 @@
 package cdi
 
 import (
+	"os"
 	"runtime"
 	"testing"
 
@@ -314,6 +315,9 @@ func TestValidateContainerEdits(t *testing.T) {
 func TestApplyContainerEdits(t *testing.T) {
 	nullDeviceMajor := int64(1)
 	nullDeviceMinor := int64(3)
+
+	mode := uint32(0666)
+	nullDeviceFileMode := (*os.FileMode)(&mode)
 	if runtime.GOOS == "darwin" {
 		nullDeviceMajor = 3
 		nullDeviceMinor = 2
@@ -360,10 +364,11 @@ func TestApplyContainerEdits(t *testing.T) {
 				Linux: &oci.Linux{
 					Devices: []oci.LinuxDevice{
 						{
-							Path:  "/dev/null",
-							Type:  "c",
-							Major: nullDeviceMajor,
-							Minor: nullDeviceMinor,
+							Path:     "/dev/null",
+							Type:     "c",
+							Major:    nullDeviceMajor,
+							Minor:    nullDeviceMinor,
+							FileMode: nullDeviceFileMode,
 						},
 					},
 					Resources: &oci.LinuxResources{
@@ -403,10 +408,11 @@ func TestApplyContainerEdits(t *testing.T) {
 				Linux: &oci.Linux{
 					Devices: []oci.LinuxDevice{
 						{
-							Path:  "/dev/null",
-							Type:  "c",
-							Major: nullDeviceMajor,
-							Minor: nullDeviceMinor,
+							Path:     "/dev/null",
+							Type:     "c",
+							Major:    nullDeviceMajor,
+							Minor:    nullDeviceMinor,
+							FileMode: nullDeviceFileMode,
 						},
 					},
 					Resources: &oci.LinuxResources{

pkg/cdi/container-edits_unix.go

@@ -21,6 +21,7 @@ package cdi
 import (
 	"errors"
 	"fmt"
+	"os"
 
 	"golang.org/x/sys/unix"
 )
@@ -31,16 +32,28 @@ const (
 	fifoDevice  = "p"
 )
 
+type deviceInfo struct {
+	// cgroup properties
+	deviceType string
+	major      int64
+	minor      int64
+
+	// device node properties
+	fileMode os.FileMode
+}
+
 // deviceInfoFromPath takes the path to a device and returns its type,
 // major and minor device numbers.
 //
 // It was adapted from https://github.com/opencontainers/runc/blob/v1.1.9/libcontainer/devices/device_unix.go#L30-L69
-func deviceInfoFromPath(path string) (devType string, major, minor int64, _ error) {
+func deviceInfoFromPath(path string) (*deviceInfo, error) {
 	var stat unix.Stat_t
 	err := unix.Lstat(path, &stat)
 	if err != nil {
-		return "", 0, 0, err
+		return nil, err
 	}
+
+	var devType string
 	switch stat.Mode & unix.S_IFMT {
 	case unix.S_IFBLK:
 		devType = blockDevice
@@ -49,38 +62,71 @@ func deviceInfoFromPath(path string) (devType string, major, minor int64, _ erro
 	case unix.S_IFIFO:
 		devType = fifoDevice
 	default:
-		return "", 0, 0, errors.New("not a device node")
+		return nil, errors.New("not a device node")
 	}
 	devNumber := uint64(stat.Rdev) //nolint:unconvert // Rdev is uint32 on e.g. MIPS.
-	return devType, int64(unix.Major(devNumber)), int64(unix.Minor(devNumber)), nil
+
+	di := deviceInfo{
+		deviceType: devType,
+		major:      int64(unix.Major(devNumber)),
+		minor:      int64(unix.Minor(devNumber)),
+		fileMode:   os.FileMode(stat.Mode &^ unix.S_IFMT),
+	}
+
+	return &di, nil
 }
 
 // fillMissingInfo fills in missing mandatory attributes from the host device.
 func (d *DeviceNode) fillMissingInfo() error {
+	hasMinimalSpecification := d.Type != "" && (d.Major != 0 || d.Type == fifoDevice)
+
+	// Ensure that the host path and the container path match.
 	if d.HostPath == "" {
 		d.HostPath = d.Path
 	}
 
-	if d.Type != "" && (d.Major != 0 || d.Type == fifoDevice) {
+	// Try to extract the device info from the host path.
+	di, err := deviceInfoFromPath(d.HostPath)
+	if err != nil {
+		// The error is only considered fatal if the device is not already
+		// minimally specified since it is allowed for a device vendor to fully
+		// specify a device node specification.
+		if !hasMinimalSpecification {
+			return fmt.Errorf("failed to stat CDI host device %q: %w", d.HostPath, err)
+		}
 		return nil
 	}
 
-	deviceType, major, minor, err := deviceInfoFromPath(d.HostPath)
-	if err != nil {
-		return fmt.Errorf("failed to stat CDI host device %q: %w", d.HostPath, err)
+	// Even for minimally-specified device nodes, we update the file mode if
+	// required. This is useful for rootless containers where device node
+	// requests may be treated as bind mounts.
+	if d.FileMode == nil {
+		d.FileMode = &di.fileMode
+	}
+
+	// If the device is minimally specified, we make no further updates and
+	// don't perform additional checks.
+	if hasMinimalSpecification {
+		return nil
 	}
 
 	if d.Type == "" {
-		d.Type = deviceType
-	} else {
-		if d.Type != deviceType {
-			return fmt.Errorf("CDI device (%q, %q), host type mismatch (%s, %s)",
-				d.Path, d.HostPath, d.Type, deviceType)
-		}
+		d.Type = di.deviceType
 	}
-	if d.Major == 0 && d.Type != fifoDevice {
-		d.Major = major
-		d.Minor = minor
+	if d.Type != di.deviceType {
+		return fmt.Errorf("CDI device (%q, %q), host type mismatch (%s, %s)",
+			d.Path, d.HostPath, d.Type, di.deviceType)
+	}
+
+	// For a fifoDevice, we do not update the major and minor number.
+	if d.Type == fifoDevice {
+		return nil
+	}
+
+	// Update the major and minor number for the device node if required.
+	if d.Major == 0 {
+		d.Major = di.major
+		d.Minor = di.minor
 	}
 
 	return nil