-
Notifications
You must be signed in to change notification settings - Fork 365
fix(ldconfig): suppress /proc mount error for non-root users #1164
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix(ldconfig): suppress /proc mount error for non-root users #1164
Conversation
Non-root users are not permitted to mount /proc, which causes `mountProc` to fail and abort `prepareRoot`. To prevent this, the error is now printed to stderr instead of causing a failure, allowing continued execution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR suppresses the error from mounting /proc for non-root users by printing the error to stderr instead of aborting execution.
- Changed error handling in prepareRoot by replacing the error return with printing to stderr
- Ensures continued execution when a non-root user attempts to mount /proc
Comments suppressed due to low confidence (1)
internal/ldconfig/ldconfig.go:134
- [nitpick] While suppressing the error for non-root users aligns with the intent of this fix, consider adding a comment clarifying any downstream effects if mountProc fails to mount /proc. This extra context could aid future debugging and maintenance.
fmt.Fprintf(os.Stderr, "error mounting /proc: %s\n", err)
@@ -130,7 +130,8 @@ func (l *Ldconfig) prepareRoot() (string, error) { | |||
// To prevent leaking the parent proc filesystem, we create a new proc mount | |||
// in the specified root. | |||
if err := mountProc(l.inRoot); err != nil { | |||
return "", fmt.Errorf("error mounting /proc: %w", err) | |||
// Non-root users cannot mount /proc; print the error but continue execution |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to detect whether this is running as a non-root user instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried some ways.
At first, I thought getting uid and gid would help. But it outcomes "0" in the containers, caused by sandboxed namespaces.
AFAIK, the only way to detect whether this is running as a non-root user is to trying it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at runc
the following check is used to detect rooless:
RootlessEUID: os.Geteuid() != 0,
In the hooks where this is used, we're already in a new userns here, so as you pointed out the check may not work here. What about checking this where the hook is actually invoked and then passing that through to the runner?
diff --git a/cmd/nvidia-cdi-hook/create-soname-symlinks/soname-symlinks.go b/cmd/nvidia-cdi-hook/create-soname-symlinks/soname-symlinks.go
index 7f1da580..10d52c43 100644
--- a/cmd/nvidia-cdi-hook/create-soname-symlinks/soname-symlinks.go
+++ b/cmd/nvidia-cdi-hook/create-soname-symlinks/soname-symlinks.go
@@ -142,22 +142,8 @@ func createSonameSymlinksHandler() {
// It is invoked from a reexec'd handler and provides namespace isolation for
// the operations performed by this hook. At the point where this is invoked,
// we are in a new mount namespace that is cloned from the parent.
-//
-// args[0] is the reexec initializer function name
-// args[1] is the path of the ldconfig binary on the host
-// args[2] is the container root directory
-// The remaining args are directories where soname symlinks need to be created.
func createSonameSymlinks(args []string) error {
- if len(args) < 3 {
- return fmt.Errorf("incorrect arguments: %v", args)
- }
- hostLdconfigPath := args[1]
- containerRootDirPath := args[2]
-
- ldconfig, err := ldconfig.New(
- hostLdconfigPath,
- containerRootDirPath,
- )
+ ldconfig, err := ldconfig.NewFromArgs(args...)
if err != nil {
return fmt.Errorf("failed to construct ldconfig runner: %w", err)
}
diff --git a/cmd/nvidia-cdi-hook/update-ldcache/update-ldcache.go b/cmd/nvidia-cdi-hook/update-ldcache/update-ldcache.go
index ef614709..78219022 100644
--- a/cmd/nvidia-cdi-hook/update-ldcache/update-ldcache.go
+++ b/cmd/nvidia-cdi-hook/update-ldcache/update-ldcache.go
@@ -140,22 +140,8 @@ func updateLdCacheHandler() {
// It is invoked from a reexec'd handler and provides namespace isolation for
// the operations performed by this hook. At the point where this is invoked,
// we are in a new mount namespace that is cloned from the parent.
-//
-// args[0] is the reexec initializer function name
-// args[1] is the path of the ldconfig binary on the host
-// args[2] is the container root directory
-// The remaining args are folders where soname symlinks need to be created.
func updateLdCache(args []string) error {
- if len(args) < 3 {
- return fmt.Errorf("incorrect arguments: %v", args)
- }
- hostLdconfigPath := args[1]
- containerRootDirPath := args[2]
-
- ldconfig, err := ldconfig.New(
- hostLdconfigPath,
- containerRootDirPath,
- )
+ ldconfig, err := ldconfig.NewFromArgs(args...)
if err != nil {
return fmt.Errorf("failed to construct ldconfig runner: %w", err)
}
diff --git a/internal/ldconfig/ldconfig.go b/internal/ldconfig/ldconfig.go
index f3db1a77..86324a82 100644
--- a/internal/ldconfig/ldconfig.go
+++ b/internal/ldconfig/ldconfig.go
@@ -19,9 +19,11 @@ package ldconfig
import (
"fmt"
+ "log"
"os"
"os/exec"
"path/filepath"
+ "strconv"
"strings"
"github.com/NVIDIA/nvidia-container-toolkit/internal/config"
@@ -39,6 +41,7 @@ const (
type Ldconfig struct {
ldconfigPath string
inRoot string
+ isRootless bool
}
// NewRunner creates an exec.Cmd that can be used to run ldconfig.
@@ -47,25 +50,47 @@ func NewRunner(id string, ldconfigPath string, containerRoot string, additionala
id,
strings.TrimPrefix(config.NormalizeLDConfigPath("@"+ldconfigPath), "@"),
containerRoot,
+ fmt.Sprintf("rootless=%v", os.Geteuid() != 0),
}
args = append(args, additionalargs...)
return createReexecCommand(args)
}
-// New creates an Ldconfig struct that is used to perform operations on the
-// ldcache and libraries in a particular root (e.g. a container).
-func New(ldconfigPath string, inRoot string) (*Ldconfig, error) {
- l := &Ldconfig{
- ldconfigPath: ldconfigPath,
- inRoot: inRoot,
- }
+// NewFromRunnerArgs creates an Ldconfig struct from the args passed to the Cmd
+// above.
+// This struct is used to perform operations on the ldcache and libraries in a
+// particular root (e.g. a container).
+//
+// args[0] is the reexec initializer function name
+// args[1] is the path of the ldconfig binary on the host
+// args[2] is the container root directory
+// args[3] is the flag indicating whether the container is being run rootless.
+// The remaining args are folders where soname symlinks need to be created.
+func NewFromArgs(args ...string) (*Ldconfig, error) {
+ if len(args) < 4 {
+ return nil, fmt.Errorf("incorrect arguments: %v", args)
+ }
+ ldconfigPath := args[1]
if ldconfigPath == "" {
return nil, fmt.Errorf("an ldconfig path must be specified")
}
+
+ inRoot := args[2]
if inRoot == "" || inRoot == "/" {
return nil, fmt.Errorf("ldconfig must be run in the non-system root")
}
+
+ isRootless, err := strconv.ParseBool(strings.TrimPrefix(args[3], "rootless="))
+ if err != nil {
+ return nil, err
+ }
+
+ l := &Ldconfig{
+ ldconfigPath: ldconfigPath,
+ inRoot: inRoot,
+ isRootless: isRootless,
+ }
return l, nil
}
@@ -130,7 +155,10 @@ func (l *Ldconfig) prepareRoot() (string, error) {
// To prevent leaking the parent proc filesystem, we create a new proc mount
// in the specified root.
if err := mountProc(l.inRoot); err != nil {
- return "", fmt.Errorf("error mounting /proc: %w", err)
+ if !l.isRootless {
+ return "", fmt.Errorf("error mounting /proc: %w", err)
+ }
+ log.Printf("Ignoring error for rootless container: %v", err)
}
// We mount the host ldconfig before we pivot root since host paths are not
Non-root users are not permitted to mount /proc, which causes
mountProc
to fail and abortprepareRoot
. To prevent this, the error is now printed to stderr instead of causing a failure, allowing continued execution.Fixes
Fixes #1163