Merge branch 'ctk-hook-chmod-improve-eperm-handling' into 'main'

nvidia-ctk hook chmod: Improve permission error handling

See merge request nvidia/container-toolkit/container-toolkit!496

cmd/nvidia-ctk/hook/chmod/chmod.go

@@ -17,14 +17,15 @@
 package chmod
 
 import (
+	"errors"
 	"fmt"
+	"io/fs"
 	"os"
 	"path/filepath"
+	"strconv"
 	"strings"
-	"syscall"
 
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
-	"github.com/NVIDIA/nvidia-container-toolkit/internal/lookup"
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/oci"
 	"github.com/urfave/cli/v2"
 )
@@ -35,7 +36,8 @@ type command struct {
 
 type config struct {
 	paths         cli.StringSlice
-	mode          string
+	modeStr       string
+	mode          fs.FileMode
 	containerSpec string
 }
 
@@ -72,7 +74,7 @@ func (m command) build() *cli.Command {
 		&cli.StringFlag{
 			Name:        "mode",
 			Usage:       "Specify the file mode",
-			Destination: &cfg.mode,
+			Destination: &cfg.modeStr,
 		},
 		&cli.StringFlag{
 			Name:        "container-spec",
@@ -85,10 +87,16 @@ func (m command) build() *cli.Command {
 }
 
 func validateFlags(c *cli.Context, cfg *config) error {
-	if strings.TrimSpace(cfg.mode) == "" {
+	if strings.TrimSpace(cfg.modeStr) == "" {
 		return fmt.Errorf("a non-empty mode must be specified")
 	}
 
+	modeInt, err := strconv.ParseUint(cfg.modeStr, 8, 32)
+	if err != nil {
+		return fmt.Errorf("failed to parse mode as octal: %v", err)
+	}
+	cfg.mode = fs.FileMode(modeInt)
+
 	for _, p := range cfg.paths.Value() {
 		if strings.TrimSpace(p) == "" {
 			return fmt.Errorf("paths must not be empty")
@@ -112,34 +120,38 @@ func (m command) run(c *cli.Context, cfg *config) error {
 		return fmt.Errorf("empty container root detected")
 	}
 
-	paths := m.getPaths(containerRoot, cfg.paths.Value())
+	paths := m.getPaths(containerRoot, cfg.paths.Value(), cfg.mode)
 	if len(paths) == 0 {
 		m.logger.Debugf("No paths specified; exiting")
 		return nil
 	}
 
-	locator := lookup.NewExecutableLocator(m.logger, "")
+	for _, path := range paths {
-	targets, err := locator.Locate("chmod")
+		err = os.Chmod(path, cfg.mode)
-	if err != nil {
+		// in some cases this is not an issue (e.g. whole /dev mounted), see #143
-		return fmt.Errorf("failed to locate chmod: %v", err)
+		if errors.Is(err, fs.ErrPermission) {
+			m.logger.Debugf("Ignoring permission error with chmod: %v", err)
+			err = nil
+		}
 	}
-	chmodPath := targets[0]
 
-	args := append([]string{filepath.Base(chmodPath), cfg.mode}, paths...)
+	return err
-
-	//nolint:gosec // TODO: Can we harden this so that there is less risk of command injection
-	return syscall.Exec(chmodPath, args, nil)
 }
 
 // getPaths updates the specified paths relative to the root.
-func (m command) getPaths(root string, paths []string) []string {
+func (m command) getPaths(root string, paths []string, desiredMode fs.FileMode) []string {
 	var pathsInRoot []string
 	for _, f := range paths {
 		path := filepath.Join(root, f)
-		if _, err := os.Stat(path); err != nil {
+		stat, err := os.Stat(path)
+		if err != nil {
 			m.logger.Debugf("Skipping path %q: %v", path, err)
 			continue
 		}
+		if (stat.Mode()&(fs.ModePerm|fs.ModeSetuid|fs.ModeSetgid|fs.ModeSticky))^desiredMode == 0 {
+			m.logger.Debugf("Skipping path %q: already desired mode", path)
+			continue
+		}
 		pathsInRoot = append(pathsInRoot, path)
 	}