Skip to content

Commit

Permalink
runsc: always run the sandbox process in a new pid namespace
Browse files Browse the repository at this point in the history
The sandbox process was executed in the current pid namespace if a target
platform used ptrace. It was the workaround for the kernel issue that was
fixed by 8fb335e07837 ("kernel/exit.c: release ptraced tasks before
zap_pid_ns_processes"). This fix was back-ported to stable branches.

PiperOrigin-RevId: 668121544
  • Loading branch information
avagin authored and gvisor-bot committed Aug 27, 2024
1 parent f02d783 commit cc1f550
Show file tree
Hide file tree
Showing 6 changed files with 6 additions and 34 deletions.
3 changes: 0 additions & 3 deletions pkg/sentry/platform/platform.go
Original file line number Diff line number Diff line change
Expand Up @@ -455,9 +455,6 @@ func (f SegmentationFault) Error() string {

// Requirements is used to specify platform specific requirements.
type Requirements struct {
// RequiresCurrentPIDNS indicates that the sandbox has to be started in the
// current pid namespace.
RequiresCurrentPIDNS bool
// RequiresCapSysPtrace indicates that the sandbox has to be started with
// the CAP_SYS_PTRACE capability.
RequiresCapSysPtrace bool
Expand Down
3 changes: 0 additions & 3 deletions pkg/sentry/platform/ptrace/ptrace.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,11 +271,8 @@ func (*constructor) OpenDevice(_ string) (*fd.FD, error) {

// Flags implements platform.Constructor.Flags().
func (*constructor) Requirements() platform.Requirements {
// TODO(b/75837838): Also set a new PID namespace so that we limit
// access to other host processes.
return platform.Requirements{
RequiresCapSysPtrace: true,
RequiresCurrentPIDNS: true,
}
}

Expand Down
3 changes: 0 additions & 3 deletions pkg/sentry/platform/systrap/systrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -410,11 +410,8 @@ func (*constructor) OpenDevice(_ string) (*fd.FD, error) {

// Requirements implements platform.Constructor.Requirements().
func (*constructor) Requirements() platform.Requirements {
// TODO(b/75837838): Also set a new PID namespace so that we limit
// access to other host processes.
return platform.Requirements{
RequiresCapSysPtrace: true,
RequiresCurrentPIDNS: true,
}
}

Expand Down
6 changes: 1 addition & 5 deletions runsc/cmd/boot.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,6 @@ type Boot struct {

saveFDs intFlags

// pidns is set if the sandbox is in its own pid namespace.
pidns bool

// attached is set to true to kill the sandbox process when the parent process
// terminates. This flag is set when the command execve's itself because
// parent death signal doesn't propagate through execve when uid/gid changes.
Expand Down Expand Up @@ -199,7 +196,6 @@ func (b *Boot) SetFlags(f *flag.FlagSet) {
f.StringVar(&b.bundleDir, "bundle", "", "required path to the root of the bundle directory")
f.BoolVar(&b.applyCaps, "apply-caps", false, "if true, apply capabilities defined in the spec to the process")
f.BoolVar(&b.setUpRoot, "setup-root", false, "if true, set up an empty root for the process")
f.BoolVar(&b.pidns, "pidns", false, "if true, the sandbox is in its own PID namespace")
f.IntVar(&b.cpuNum, "cpu-num", 0, "number of CPUs to create inside the sandbox")
f.IntVar(&b.procMountSyncFD, "proc-mount-sync-fd", -1, "file descriptor that has to be written to when /proc isn't needed anymore and can be unmounted")
f.IntVar(&b.syncUsernsFD, "sync-userns-fd", -1, "file descriptor used to synchronize rootless user namespace initialization.")
Expand Down Expand Up @@ -300,7 +296,7 @@ func (b *Boot) Execute(_ context.Context, f *flag.FlagSet, args ...any) subcomma
}

if b.setUpRoot {
if err := setUpChroot(b.pidns, spec, conf); err != nil {
if err := setUpChroot(spec, conf); err != nil {
util.Fatalf("error setting up chroot: %v", err)
}
argOverride["setup-root"] = "false"
Expand Down
14 changes: 4 additions & 10 deletions runsc/cmd/chroot.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func copyFile(dst, src string) error {

// setUpChroot creates an empty directory with runsc mounted at /runsc and proc
// mounted at /proc.
func setUpChroot(pidns bool, spec *specs.Spec, conf *config.Config) error {
func setUpChroot(spec *specs.Spec, conf *config.Config) error {
// We are a new mount namespace, so we can use /tmp as a directory to
// construct a new root.
chroot := os.TempDir()
Expand All @@ -108,15 +108,9 @@ func setUpChroot(pidns bool, spec *specs.Spec, conf *config.Config) error {
log.Warningf("Failed to copy /etc/localtime: %v. UTC timezone will be used.", err)
}

if pidns {
flags := uint32(unix.MS_NOSUID | unix.MS_NODEV | unix.MS_NOEXEC | unix.MS_RDONLY)
if err := mountInChroot(chroot, "proc", "/proc", "proc", flags); err != nil {
return fmt.Errorf("error mounting proc in chroot: %v", err)
}
} else {
if err := mountInChroot(chroot, "/proc", "/proc", "bind", unix.MS_BIND|unix.MS_RDONLY|unix.MS_REC); err != nil {
return fmt.Errorf("error mounting proc in chroot: %v", err)
}
flags := uint32(unix.MS_NOSUID | unix.MS_NODEV | unix.MS_NOEXEC | unix.MS_RDONLY)
if err := mountInChroot(chroot, "proc", "/proc", "proc", flags); err != nil {
return fmt.Errorf("error mounting proc in chroot: %v", err)
}

if err := tpuProxyUpdateChroot(chroot, spec, conf); err != nil {
Expand Down
11 changes: 1 addition & 10 deletions runsc/sandbox/sandbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -946,16 +946,7 @@ func (s *Sandbox) createSandboxProcess(conf *config.Config, args *Args, startSyn
{Type: specs.IPCNamespace},
{Type: specs.MountNamespace},
{Type: specs.UTSNamespace},
}

if gPlatform.Requirements().RequiresCurrentPIDNS {
// TODO(b/75837838): Also set a new PID namespace so that we limit
// access to other host processes.
log.Infof("Sandbox will be started in the current PID namespace")
} else {
log.Infof("Sandbox will be started in a new PID namespace")
nss = append(nss, specs.LinuxNamespace{Type: specs.PIDNamespace})
cmd.Args = append(cmd.Args, "--pidns=true")
{Type: specs.PIDNamespace},
}

if specutils.NVProxyEnabled(args.Spec, conf) {
Expand Down

0 comments on commit cc1f550

Please sign in to comment.