From 39bd7b7217db91a9b349738a03960ed7b9390327 Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Thu, 27 Jan 2022 10:35:38 -0800
Subject: [PATCH] libct: Container, Factory: rm newuidmap/newgidmap

These were introduced in commit d8b669400 back in 2017, with a TODO
of "make binary names configurable". Apparently, everyone is happy with
the hardcoded names. In fact, they *are* configurable (by prepending the
PATH with a directory containing own version of newuidmap/newgidmap).

Now, these binaries are only needed in a few specific cases (when
rootless is set etc.), so let's look them up only when needed.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 libcontainer/container_linux.go | 29 +++++++++++++++++------------
 libcontainer/factory_linux.go   | 27 ---------------------------
 utils_linux.go                  | 17 +----------------
 3 files changed, 18 insertions(+), 55 deletions(-)

diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go
index 6e0a5cefe..b285a3175 100644
--- a/libcontainer/container_linux.go
+++ b/libcontainer/container_linux.go
@@ -43,8 +43,6 @@ type linuxContainer struct {
 	intelRdtManager      intelrdt.Manager
 	initProcess          parentProcess
 	initProcessStartTime uint64
-	newuidmapPath        string
-	newgidmapPath        string
 	m                    sync.Mutex
 	criuVersion          int
 	state                containerState
@@ -2148,11 +2146,16 @@ func (c *linuxContainer) bootstrapData(cloneFlags uintptr, nsMaps map[configs.Na
 	if !joinExistingUser {
 		// write uid mappings
 		if len(c.config.UidMappings) > 0 {
-			if c.config.RootlessEUID && c.newuidmapPath != "" {
-				r.AddData(&Bytemsg{
-					Type:  UidmapPathAttr,
-					Value: []byte(c.newuidmapPath),
-				})
+			if c.config.RootlessEUID {
+				// We resolve the paths for new{u,g}idmap from
+				// the context of runc to avoid doing a path
+				// lookup in the nsexec context.
+				if path, err := exec.LookPath("newuidmap"); err == nil {
+					r.AddData(&Bytemsg{
+						Type:  UidmapPathAttr,
+						Value: []byte(path),
+					})
+				}
 			}
 			b, err := encodeIDMapping(c.config.UidMappings)
 			if err != nil {
@@ -2174,11 +2177,13 @@ func (c *linuxContainer) bootstrapData(cloneFlags uintptr, nsMaps map[configs.Na
 				Type:  GidmapAttr,
 				Value: b,
 			})
-			if c.config.RootlessEUID && c.newgidmapPath != "" {
-				r.AddData(&Bytemsg{
-					Type:  GidmapPathAttr,
-					Value: []byte(c.newgidmapPath),
-				})
+			if c.config.RootlessEUID {
+				if path, err := exec.LookPath("newgidmap"); err == nil {
+					r.AddData(&Bytemsg{
+						Type:  GidmapPathAttr,
+						Value: []byte(path),
+					})
+				}
 			}
 			if requiresRootOrMappingTool(c.config) {
 				r.AddData(&Boolmsg{
diff --git a/libcontainer/factory_linux.go b/libcontainer/factory_linux.go
index 32d008a2c..afb0a43fa 100644
--- a/libcontainer/factory_linux.go
+++ b/libcontainer/factory_linux.go
@@ -84,11 +84,6 @@ type LinuxFactory struct {
 	// Root directory for the factory to store state.
 	Root string
 
-	// New{u,g}idmapPath is the path to the binaries used for mapping with
-	// rootless containers.
-	NewuidmapPath string
-	NewgidmapPath string
-
 	// NewIntelRdtManager returns an initialized Intel RDT manager for a single container.
 	NewIntelRdtManager func(config *configs.Config, id string, path string) intelrdt.Manager
 }
@@ -154,8 +149,6 @@ func (l *LinuxFactory) Create(id string, config *configs.Config) (Container, err
 		id:            id,
 		root:          containerRoot,
 		config:        config,
-		newuidmapPath: l.NewuidmapPath,
-		newgidmapPath: l.NewgidmapPath,
 		cgroupManager: cm,
 	}
 	if l.NewIntelRdtManager != nil {
@@ -195,8 +188,6 @@ func (l *LinuxFactory) Load(id string) (Container, error) {
 		initProcessStartTime: state.InitProcessStartTime,
 		id:                   id,
 		config:               &state.Config,
-		newuidmapPath:        l.NewuidmapPath,
-		newgidmapPath:        l.NewgidmapPath,
 		cgroupManager:        cm,
 		root:                 containerRoot,
 		created:              state.Created,
@@ -327,24 +318,6 @@ func (l *LinuxFactory) validateID(id string) error {
 	return nil
 }
 
-// NewuidmapPath returns an option func to configure a LinuxFactory with the
-// provided ..
-func NewuidmapPath(newuidmapPath string) func(*LinuxFactory) error {
-	return func(l *LinuxFactory) error {
-		l.NewuidmapPath = newuidmapPath
-		return nil
-	}
-}
-
-// NewgidmapPath returns an option func to configure a LinuxFactory with the
-// provided ..
-func NewgidmapPath(newgidmapPath string) func(*LinuxFactory) error {
-	return func(l *LinuxFactory) error {
-		l.NewgidmapPath = newgidmapPath
-		return nil
-	}
-}
-
 func parseMountFds() ([]int, error) {
 	fdsJSON := os.Getenv("_LIBCONTAINER_MOUNT_FDS")
 	if fdsJSON == "" {
diff --git a/utils_linux.go b/utils_linux.go
index 9f7619d65..d49b024ec 100644
--- a/utils_linux.go
+++ b/utils_linux.go
@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"net"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"strconv"
 
@@ -34,21 +33,7 @@ func loadFactory(context *cli.Context) (libcontainer.Factory, error) {
 
 	intelRdtManager := libcontainer.IntelRdtFs
 
-	// We resolve the paths for {newuidmap,newgidmap} from the context of runc,
-	// to avoid doing a path lookup in the nsexec context. TODO: The binary
-	// names are not currently configurable.
-	newuidmap, err := exec.LookPath("newuidmap")
-	if err != nil {
-		newuidmap = ""
-	}
-	newgidmap, err := exec.LookPath("newgidmap")
-	if err != nil {
-		newgidmap = ""
-	}
-
-	return libcontainer.New(abs, intelRdtManager,
-		libcontainer.NewuidmapPath(newuidmap),
-		libcontainer.NewgidmapPath(newgidmap))
+	return libcontainer.New(abs, intelRdtManager)
 }
 
 // getContainer returns the specified container instance by loading it from state