Add support for injecting additional GIDs

This change adds support for injecting additional GIDs using the internal
CDI representations. (Applicable for Tegra-based systems and /dev/dri devices)
This is disabled by default, but can be opted in to by setting the
features.allow-additional-gids feature flag.

This can also be done by running

sudo nvidia-ctk config --in-place --set features.allow-additional-gids

Signed-off-by: Evan Lezar <elezar@nvidia.com>

Author: elezar

internal/config/features.go

@@ -19,10 +19,11 @@ package config
 type featureName string
 
 const (
-	FeatureGDS      = featureName("gds")
-	FeatureMOFED    = featureName("mofed")
-	FeatureNVSWITCH = featureName("nvswitch")
-	FeatureGDRCopy  = featureName("gdrcopy")
+	FeatureGDS                 = featureName("gds")
+	FeatureMOFED               = featureName("mofed")
+	FeatureNVSWITCH            = featureName("nvswitch")
+	FeatureGDRCopy             = featureName("gdrcopy")
+	FeatureAllowAdditionalGIDs = featureName("allow-additional-gids")
 )
 
 // features specifies a set of named features.
@@ -31,6 +32,11 @@ type features struct {
 	MOFED    *feature `toml:"mofed,omitempty"`
 	NVSWITCH *feature `toml:"nvswitch,omitempty"`
 	GDRCopy  *feature `toml:"gdrcopy,omitempty"`
+	// AllowAdditionalGIDs triggers the additionalGIDs field in internal CDI
+	// specifications to be populated if required. This can be useful when
+	// running the container as a user id that may not have access to device
+	// nodes.
+	AllowAdditionalGIDs *feature `toml:"allow-additional-gids,omitempty"`
 }
 
 type feature bool
@@ -40,10 +46,11 @@ type feature bool
 // variables can also be supplied.
 func (fs features) IsEnabled(n featureName, in ...getenver) bool {
 	featureEnvvars := map[featureName]string{
-		FeatureGDS:      "NVIDIA_GDS",
-		FeatureMOFED:    "NVIDIA_MOFED",
-		FeatureNVSWITCH: "NVIDIA_NVSWITCH",
-		FeatureGDRCopy:  "NVIDIA_GDRCOPY",
+		FeatureGDS:                 "NVIDIA_GDS",
+		FeatureMOFED:               "NVIDIA_MOFED",
+		FeatureNVSWITCH:            "NVIDIA_NVSWITCH",
+		FeatureGDRCopy:             "NVIDIA_GDRCOPY",
+		FeatureAllowAdditionalGIDs: "NVIDIA_ALLOW_ADDITIONAL_GIDS",
 	}
 
 	envvar := featureEnvvars[n]
@@ -56,6 +63,8 @@ func (fs features) IsEnabled(n featureName, in ...getenver) bool {
 		return fs.NVSWITCH.isEnabled(envvar, in...)
 	case FeatureGDRCopy:
 		return fs.GDRCopy.isEnabled(envvar, in...)
+	case FeatureAllowAdditionalGIDs:
+		return fs.AllowAdditionalGIDs.isEnabled(envvar, in...)
 	default:
 		return false
 	}

internal/edits/device.go

@@ -17,6 +17,9 @@
 package edits
 
 import (
+	"os"
+
+	"golang.org/x/sys/unix"
 	"tags.cncf.io/container-device-interface/pkg/cdi"
 	"tags.cncf.io/container-device-interface/specs-go"
 
@@ -26,15 +29,23 @@ import (
 type device discover.Device
 
 // toEdits converts a discovered device to CDI Container Edits.
-func (d device) toEdits() (*cdi.ContainerEdits, error) {
+func (d device) toEdits(allowAdditionalGIDs bool) (*cdi.ContainerEdits, error) {
 	deviceNode, err := d.toSpec()
 	if err != nil {
 		return nil, err
 	}
 
+	var additionalGIDs []uint32
+	if allowAdditionalGIDs {
+		if requiredGID := d.getRequiredGID(); requiredGID != 0 {
+			additionalGIDs = append(additionalGIDs, requiredGID)
+		}
+	}
+
 	e := cdi.ContainerEdits{
 		ContainerEdits: &specs.ContainerEdits{
-			DeviceNodes: []*specs.DeviceNode{deviceNode},
+			DeviceNodes:    []*specs.DeviceNode{deviceNode},
+			AdditionalGIDs: additionalGIDs,
 		},
 	}
 	return &e, nil
@@ -52,10 +63,37 @@ func (d device) toSpec() (*specs.DeviceNode, error) {
 	if hostPath == d.Path {
 		hostPath = ""
 	}
+
 	s := specs.DeviceNode{
 		HostPath: hostPath,
 		Path:     d.Path,
 	}
 
 	return &s, nil
 }
+
+// getRequiredGID returns the group id of the device if the device is not world read/writable.
+// If the information cannot be extracted or an error occurs, 0 is returned.
+func (d device) getRequiredGID() uint32 {
+	path := d.HostPath
+	if path == "" {
+		path = d.Path
+	}
+	if path == "" {
+		return 0
+	}
+
+	var stat unix.Stat_t
+	if err := unix.Lstat(path, &stat); err != nil {
+		return 0
+	}
+	// This is only supported for char devices
+	if stat.Mode&unix.S_IFMT != unix.S_IFCHR {
+		return 0
+	}
+
+	if permissionsForOther := os.FileMode(stat.Mode).Perm(); permissionsForOther&06 == 0 {
+		return stat.Gid
+	}
+	return 0
+}

internal/edits/lib.go

@@ -58,7 +58,7 @@ func (o *options) EditsFromDiscoverer(d discover.Discover) (*cdi.ContainerEdits,
 
 	c := NewContainerEdits()
 	for _, d := range devices {
-		edits, err := device(d).toEdits()
+		edits, err := device(d).toEdits(o.allowAdditionalGIDs)
 		if err != nil {
 			return nil, fmt.Errorf("failed to create container edits for device: %w", err)
 		}

internal/edits/options.go

@@ -19,7 +19,8 @@ package edits
 import "github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
 
 type options struct {
-	logger logger.Interface
+	logger              logger.Interface
+	allowAdditionalGIDs bool
 }
 
 type Option func(*options)
@@ -29,3 +30,9 @@ func WithLogger(logger logger.Interface) Option {
 		o.logger = logger
 	}
 }
+
+func WithAllowAdditionalGIDs(allowAdditionalGIDs bool) Option {
+	return func(o *options) {
+		o.allowAdditionalGIDs = allowAdditionalGIDs
+	}
+}

internal/modifier/discover.go

@@ -28,16 +28,18 @@ import (
 )
 
 type discoverModifier struct {
-	logger     logger.Interface
-	discoverer discover.Discover
+	logger              logger.Interface
+	discoverer          discover.Discover
+	allowAdditionalGIDs bool
 }
 
 // NewModifierFromDiscoverer creates a modifier that applies the discovered
 // modifications to an OCI spec if required by the runtime wrapper.
-func NewModifierFromDiscoverer(logger logger.Interface, d discover.Discover) (oci.SpecModifier, error) {
+func NewModifierFromDiscoverer(logger logger.Interface, d discover.Discover, allowAdditionalGIDs bool) (oci.SpecModifier, error) {
 	m := discoverModifier{
-		logger:     logger,
-		discoverer: d,
+		logger:              logger,
+		discoverer:          d,
+		allowAdditionalGIDs: allowAdditionalGIDs,
 	}
 	return &m, nil
 }
@@ -47,6 +49,7 @@ func NewModifierFromDiscoverer(logger logger.Interface, d discover.Discover) (oc
 func (m discoverModifier) Modify(spec *specs.Spec) error {
 	e := edits.New(
 		edits.WithLogger(m.logger),
+		edits.WithAllowAdditionalGIDs(m.allowAdditionalGIDs),
 	)
 
 	specEdits, err := e.SpecModifierFromDiscoverer(m.discoverer)

internal/modifier/discover_test.go

@@ -132,7 +132,7 @@ func TestDiscoverModifier(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.description, func(t *testing.T) {
-			m, err := NewModifierFromDiscoverer(logger, tc.discover)
+			m, err := NewModifierFromDiscoverer(logger, tc.discover, true)
 			require.NoError(t, err)
 
 			err = m.Modify(tc.spec)

internal/modifier/gated.go

@@ -78,5 +78,7 @@ func NewFeatureGatedModifier(logger logger.Interface, cfg *config.Config, image
 		discoverers = append(discoverers, d)
 	}
 
-	return NewModifierFromDiscoverer(logger, discover.Merge(discoverers...))
+	allowAdditionalGIDs := !cfg.Features.IsEnabled(config.FeatureAllowAdditionalGIDs, image)
+
+	return NewModifierFromDiscoverer(logger, discover.Merge(discoverers...), allowAdditionalGIDs)
 }

internal/modifier/graphics.go

@@ -62,7 +62,9 @@ func NewGraphicsModifier(logger logger.Interface, cfg *config.Config, image imag
 		drmNodes,
 		mounts,
 	)
-	return NewModifierFromDiscoverer(logger, d)
+
+	allowAdditionalGIDs := cfg.Features.IsEnabled(config.FeatureAllowAdditionalGIDs, image)
+	return NewModifierFromDiscoverer(logger, d, allowAdditionalGIDs)
 }
 
 // requiresGraphicsModifier determines whether a graphics modifier is required.

internal/oci/spec_mock.go

@@ -65,6 +65,8 @@ type nvcdilib struct {
 	infolib info.Interface
 
 	mergedDeviceOptions []transform.MergedDeviceOption
+
+	allowAdditionalGIDs bool
 }
 
 // New creates a new nvcdi library
@@ -196,6 +198,7 @@ func (m *wrapper) GetCommonEdits() (*cdi.ContainerEdits, error) {
 func (l *nvcdilib) editsFromDiscoverer(d discover.Discover) (*cdi.ContainerEdits, error) {
 	e := edits.New(
 		edits.WithLogger(l.logger),
+		edits.WithAllowAdditionalGIDs(l.allowAdditionalGIDs),
 	)
 	return e.EditsFromDiscoverer(d)
 }