Cleanup around urpc file payload handling

urpc always closes all files once the RPC function returns.

PiperOrigin-RevId: 248406857
Change-Id: I400a8562452ec75c8e4bddc2154948567d572950

runsc/boot/controller.go

@@ -211,12 +211,6 @@ type StartArgs struct {
 func (cm *containerManager) Start(args *StartArgs, _ *struct{}) error {
 	log.Debugf("containerManager.Start: %+v", args)
 
-	defer func() {
-		for _, f := range args.FilePayload.Files {
-			f.Close()
-		}
-	}()
-
 	// Validate arguments.
 	if args == nil {
 		return errors.New("start missing arguments")
@@ -305,21 +299,19 @@ type RestoreOpts struct {
 func (cm *containerManager) Restore(o *RestoreOpts, _ *struct{}) error {
 	log.Debugf("containerManager.Restore")
 
-	var specFile *os.File
-	deviceFD := -1
+	var specFile, deviceFile *os.File
 	switch numFiles := len(o.FilePayload.Files); numFiles {
 	case 2:
-		var err error
 		// The device file is donated to the platform.
 		// Can't take ownership away from os.File. dup them to get a new FD.
-		deviceFD, err = syscall.Dup(int(o.FilePayload.Files[1].Fd()))
+		fd, err := syscall.Dup(int(o.FilePayload.Files[1].Fd()))
 		if err != nil {
 			return fmt.Errorf("failed to dup file: %v", err)
 		}
+		deviceFile = os.NewFile(uintptr(fd), "platform device")
 		fallthrough
 	case 1:
 		specFile = o.FilePayload.Files[0]
-		defer specFile.Close()
 	case 0:
 		return fmt.Errorf("at least one file must be passed to Restore")
 	default:
@@ -331,7 +323,7 @@ func (cm *containerManager) Restore(o *RestoreOpts, _ *struct{}) error {
 	cm.l.k.Pause()
 	cm.l.k.Destroy()
 
-	p, err := createPlatform(cm.l.conf, deviceFD)
+	p, err := createPlatform(cm.l.conf, deviceFile)
 	if err != nil {
 		return fmt.Errorf("creating platform: %v", err)
 	}
@@ -357,7 +349,7 @@ func (cm *containerManager) Restore(o *RestoreOpts, _ *struct{}) error {
 	if eps, ok := networkStack.(*epsocket.Stack); ok {
 		stack.StackFromEnv = eps.Stack // FIXME(b/36201077)
 	}
-	info, err := o.FilePayload.Files[0].Stat()
+	info, err := specFile.Stat()
 	if err != nil {
 		return err
 	}
@@ -366,9 +358,7 @@ func (cm *containerManager) Restore(o *RestoreOpts, _ *struct{}) error {
 	}
 
 	// Load the state.
-	loadOpts := state.LoadOpts{
-		Source: o.FilePayload.Files[0],
-	}
+	loadOpts := state.LoadOpts{Source: specFile}
 	if err := loadOpts.Load(k, networkStack); err != nil {
 		return err
 	}

runsc/boot/loader.go

@@ -152,8 +152,8 @@ type Args struct {
 	Conf *Config
 	// ControllerFD is the FD to the URPC controller.
 	ControllerFD int
-	// DeviceFD is an optional argument that is passed to the platform.
-	DeviceFD int
+	// Device is an optional argument that is passed to the platform.
+	Device *os.File
 	// GoferFDs is an array of FDs used to connect with the Gofer.
 	GoferFDs []int
 	// StdioFDs is the stdio for the application.
@@ -183,7 +183,7 @@ func New(args Args) (*Loader, error) {
 	}
 
 	// Create kernel and platform.
-	p, err := createPlatform(args.Conf, args.DeviceFD)
+	p, err := createPlatform(args.Conf, args.Device)
 	if err != nil {
 		return nil, fmt.Errorf("creating platform: %v", err)
 	}
@@ -401,17 +401,17 @@ func (l *Loader) Destroy() {
 	l.watchdog.Stop()
 }
 
-func createPlatform(conf *Config, deviceFD int) (platform.Platform, error) {
+func createPlatform(conf *Config, deviceFile *os.File) (platform.Platform, error) {
 	switch conf.Platform {
 	case PlatformPtrace:
 		log.Infof("Platform: ptrace")
 		return ptrace.New()
 	case PlatformKVM:
 		log.Infof("Platform: kvm")
-		if deviceFD < 0 {
-			return nil, fmt.Errorf("kvm device FD must be provided")
+		if deviceFile == nil {
+			return nil, fmt.Errorf("kvm device file must be provided")
 		}
-		return kvm.New(os.NewFile(uintptr(deviceFD), "kvm device"))
+		return kvm.New(deviceFile)
 	default:
 		return nil, fmt.Errorf("invalid platform %v", conf.Platform)
 	}
@@ -590,18 +590,22 @@ func (l *Loader) startContainer(k *kernel.Kernel, spec *specs.Spec, conf *Config
 		return fmt.Errorf("creating new process: %v", err)
 	}
 
+	// setupContainerFS() dups stdioFDs, so we don't need to dup them here.
+	var stdioFDs []int
+	for _, f := range files[:3] {
+		stdioFDs = append(stdioFDs, int(f.Fd()))
+	}
+
 	// Can't take ownership away from os.File. dup them to get a new FDs.
-	var ioFDs []int
-	for _, f := range files {
+	var goferFDs []int
+	for _, f := range files[3:] {
 		fd, err := syscall.Dup(int(f.Fd()))
 		if err != nil {
 			return fmt.Errorf("failed to dup file: %v", err)
 		}
-		ioFDs = append(ioFDs, fd)
+		goferFDs = append(goferFDs, fd)
 	}
 
-	stdioFDs := ioFDs[:3]
-	goferFDs := ioFDs[3:]
 	if err := setupContainerFS(
 		&procArgs,
 		spec,
@@ -616,13 +620,6 @@ func (l *Loader) startContainer(k *kernel.Kernel, spec *specs.Spec, conf *Config
 		return fmt.Errorf("configuring container FS: %v", err)
 	}
 
-	// setFileSystemForProcess dup'd stdioFDs, so we can close them.
-	for i, fd := range stdioFDs {
-		if err := syscall.Close(fd); err != nil {
-			return fmt.Errorf("closing stdio FD #%d: %v", i, fd)
-		}
-	}
-
 	ctx := procArgs.NewContext(l.k)
 	mns := k.RootMountNamespace()
 	if err := setExecutablePath(ctx, mns, &procArgs); err != nil {

runsc/boot/loader_test.go

@@ -115,7 +115,6 @@ func createLoader() (*Loader, func(), error) {
 		Spec:         spec,
 		Conf:         conf,
 		ControllerFD: fd,
-		DeviceFD:     -1,
 		GoferFDs:     []int{sandEnd},
 		StdioFDs:     stdio,
 	}

runsc/cmd/boot.go

@@ -213,7 +213,7 @@ func (b *Boot) Execute(_ context.Context, f *flag.FlagSet, args ...interface{})
 		Spec:         spec,
 		Conf:         conf,
 		ControllerFD: b.controllerFD,
-		DeviceFD:     b.deviceFD,
+		Device:       os.NewFile(uintptr(b.deviceFD), "platform device"),
 		GoferFDs:     b.ioFDs.GetArray(),
 		StdioFDs:     b.stdioFDs.GetArray(),
 		Console:      b.console,