diff options
author | Nicolas Lacasse <nlacasse@google.com> | 2018-09-11 13:08:36 -0700 |
---|---|---|
committer | Shentubot <shentubot@google.com> | 2018-09-11 13:09:46 -0700 |
commit | 6cc9b311af3633d244f526abed50c0d3b0ce06a1 (patch) | |
tree | 923f589f98d323f17dd2a635c2744564de43f210 /runsc/sandbox | |
parent | c44bc6612fc4554d0aa4e484a46cd1f6b6a7b5c5 (diff) |
platform: Pass device fd into platform constructor.
We were previously openining the platform device (i.e. /dev/kvm) inside the
platfrom constructor (i.e. kvm.New). This requires that we have RW access to
the platform device when constructing the platform.
However, now that the runsc sandbox process runs as user "nobody", it is not
able to open the platform device.
This CL changes the kvm constructor to take the platform device FD, rather than
opening the device file itself. The device file is opened outside of the
sandbox and passed to the sandbox process.
PiperOrigin-RevId: 212505804
Change-Id: I427e1d9de5eb84c84f19d513356e1bb148a52910
Diffstat (limited to 'runsc/sandbox')
-rw-r--r-- | runsc/sandbox/BUILD | 1 | ||||
-rw-r--r-- | runsc/sandbox/chroot.go | 40 | ||||
-rw-r--r-- | runsc/sandbox/sandbox.go | 40 |
3 files changed, 43 insertions, 38 deletions
diff --git a/runsc/sandbox/BUILD b/runsc/sandbox/BUILD index 8ebd14c4e..5cf8f0cda 100644 --- a/runsc/sandbox/BUILD +++ b/runsc/sandbox/BUILD @@ -18,6 +18,7 @@ go_library( "//pkg/control/server", "//pkg/log", "//pkg/sentry/control", + "//pkg/sentry/platform/kvm", "//pkg/urpc", "//runsc/boot", "//runsc/console", diff --git a/runsc/sandbox/chroot.go b/runsc/sandbox/chroot.go index f35d9c72d..749bf3782 100644 --- a/runsc/sandbox/chroot.go +++ b/runsc/sandbox/chroot.go @@ -22,7 +22,6 @@ import ( "syscall" "gvisor.googlesource.com/gvisor/pkg/log" - "gvisor.googlesource.com/gvisor/runsc/boot" "gvisor.googlesource.com/gvisor/runsc/specutils" ) @@ -39,18 +38,12 @@ func mountInChroot(chroot, src, dst, typ string, flags uint32) error { if err := specutils.Mount(src, chrootDst, typ, flags); err != nil { return fmt.Errorf("error mounting %q at %q: %v", src, chrootDst, err) } - - // Make sure the mount is accessible to all users, since we will be - // running as nobody inside the chroot. - if err := os.Chmod(chrootDst, 0777); err != nil { - return fmt.Errorf("Chmod(%q) failed: %v", chroot, err) - } return nil } -// setUpChroot creates an empty directory with runsc mounted at /runsc, proc -// mounted at /proc, and any dev files needed for the platform. -func setUpChroot(platform boot.PlatformType) (string, error) { +// setUpChroot creates an empty directory with runsc mounted at /runsc and proc +// mounted at /proc. +func setUpChroot() (string, error) { // Create the chroot directory and make it accessible to all users. chroot, err := ioutil.TempDir("", "runsc-sandbox-chroot-") if err != nil { @@ -75,18 +68,6 @@ func setUpChroot(platform boot.PlatformType) (string, error) { return "", fmt.Errorf("error mounting runsc in chroot: %v", err) } - // Mount dev files needed for platform. - var devMount string - switch platform { - case boot.PlatformKVM: - devMount = "/dev/kvm" - } - if devMount != "" { - if err := mountInChroot(chroot, devMount, devMount, "bind", syscall.MS_BIND); err != nil { - return "", fmt.Errorf("error mounting platform device in chroot: %v", err) - } - } - return chroot, nil } @@ -105,21 +86,6 @@ func tearDownChroot(chroot string) error { return fmt.Errorf("error unmounting %q: %v", exe, err) } - // Unmount platform dev files. - devFiles := []string{"dev/kvm"} - for _, f := range devFiles { - devPath := filepath.Join(chroot, f) - if _, err := os.Stat(devPath); err != nil { - if os.IsNotExist(err) { - continue - } - return fmt.Errorf("Stat(%q) failed: %v", devPath, err) - } - if err := syscall.Unmount(devPath, 0); err != nil { - return fmt.Errorf("error unmounting %q: %v", devPath, err) - } - } - // Remove chroot directory. if err := os.RemoveAll(chroot); err != nil { return fmt.Errorf("error removing %q: %v", chroot, err) diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index f272496a1..195deda1e 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -29,6 +29,7 @@ import ( "gvisor.googlesource.com/gvisor/pkg/control/server" "gvisor.googlesource.com/gvisor/pkg/log" "gvisor.googlesource.com/gvisor/pkg/sentry/control" + "gvisor.googlesource.com/gvisor/pkg/sentry/platform/kvm" "gvisor.googlesource.com/gvisor/pkg/urpc" "gvisor.googlesource.com/gvisor/runsc/boot" "gvisor.googlesource.com/gvisor/runsc/console" @@ -140,6 +141,14 @@ func (s *Sandbox) Restore(cid string, spec *specs.Spec, conf *boot.Config, f str SandboxID: s.ID, } + // If the platform needs a device fd we must pass it in. + if deviceFile, err := deviceFileForPlatform(conf.Platform); err != nil { + return err + } else if deviceFile != nil { + defer deviceFile.Close() + opt.FilePayload.Files = append(opt.FilePayload.Files, deviceFile) + } + conn, err := s.sandboxConnect() if err != nil { return err @@ -315,6 +324,16 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund nextFD++ } + // If the platform needs a device fd we must pass it in. + if deviceFile, err := deviceFileForPlatform(conf.Platform); err != nil { + return err + } else if deviceFile != nil { + defer deviceFile.Close() + cmd.ExtraFiles = append(cmd.ExtraFiles, deviceFile) + cmd.Args = append(cmd.Args, "--device-fd="+strconv.Itoa(nextFD)) + nextFD++ + } + // Sandbox stdio defaults to current process stdio. cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout @@ -428,7 +447,7 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund log.Warningf("Running sandbox in test mode without chroot. This is only safe in tests!") } else if specutils.HasCapSysAdmin() { log.Infof("Sandbox will be started in minimal chroot") - chroot, err := setUpChroot(conf.Platform) + chroot, err := setUpChroot() if err != nil { return fmt.Errorf("error setting up chroot: %v", err) } @@ -660,3 +679,22 @@ func signalProcess(pid int, sig syscall.Signal) error { } return nil } + +// deviceFileForPlatform opens the device file for the given platform. If the +// platform does not need a device file, then nil is returned. +func deviceFileForPlatform(p boot.PlatformType) (*os.File, error) { + var ( + f *os.File + err error + ) + switch p { + case boot.PlatformKVM: + f, err = kvm.OpenDevice() + default: + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("error opening device file for platform %q: %v", p, err) + } + return f, err +} |