From 90dce68ec77382c01253a8119feb1241d7440482 Mon Sep 17 00:00:00 2001 From: B Srinivas Reddy Date: Fri, 14 Aug 2026 15:30:09 +0530 Subject: [PATCH] fix(create): defer pty setup to preserve nsenter logs Remove premature PTY creation from urunc create and defer terminal setup to the reexec process before execve, preventing swallowed nsenter errors. Also remove redundant error initialization. Fixes: #961 Signed-off-by: B Srinivas Reddy --- .github/contributors.yaml | 3 ++ cmd/urunc/create.go | 109 ++++++++++++++++++++++++-------------- 2 files changed, 72 insertions(+), 40 deletions(-) diff --git a/.github/contributors.yaml b/.github/contributors.yaml index 7f7307a3d..288f2bd4e 100644 --- a/.github/contributors.yaml +++ b/.github/contributors.yaml @@ -128,3 +128,6 @@ users: OdysseasKalaitsidis: name: Odysseas Kalaitsidis email: odysseaskalaitsides@gmail.com + srinivasr: + name: B Srinivas Reddy + email: sriniv4sreddy@gmail.com diff --git a/cmd/urunc/create.go b/cmd/urunc/create.go index 5a9e2f94d..138c7e4cf 100644 --- a/cmd/urunc/create.go +++ b/cmd/urunc/create.go @@ -88,7 +88,6 @@ var createCommand = &cli.Command{ // waits for reexec process to notify, executes CreateRuntime hooks, // sends ACK to reexec process func createUnikontainer(cmd *cli.Command, uruncCfg *unikontainers.UruncConfig) (err error) { - err = nil containerID := cmd.Args().First() if err = validateID(containerID); err != nil { return err @@ -172,44 +171,13 @@ func createUnikontainer(cmd *cli.Command, uruncCfg *unikontainers.UruncConfig) ( // The main concern is the nsenter execution before the reexec. // If anything goes wrong and we mess up with nsenter debugging // is extremely hard. - if unikontainer.Spec.Process.Terminal { - ptm, err := pty.Start(reexecCommand) - if err != nil { - err = fmt.Errorf("failed to setup pty and start reexec process: %w", err) - return err - } - defer ptm.Close() - consoleSocket := cmd.String("console-socket") - conn, err := net.Dial("unix", consoleSocket) - if err != nil { - err = fmt.Errorf("failed to dial console socket: %w", err) - return err - } - defer conn.Close() - - uc, ok := conn.(*net.UnixConn) - if !ok { - err = fmt.Errorf("failed to cast unix socket") - return err - } - defer uc.Close() - - // Send file descriptor over socket. - oob := unix.UnixRights(int(ptm.Fd())) - _, _, err = uc.WriteMsgUnix([]byte(ptm.Name()), oob, nil) - if err != nil { - err = fmt.Errorf("failed to send PTY file descriptor over socket: %w", err) - return err - } - } else { - reexecCommand.Stdin = os.Stdin - reexecCommand.Stdout = os.Stdout - reexecCommand.Stderr = os.Stderr - err := reexecCommand.Start() - if err != nil { - err = fmt.Errorf("failed to start reexec process: %w", err) - return err - } + reexecCommand.Stdin = os.Stdin + reexecCommand.Stdout = os.Stdout + reexecCommand.Stderr = os.Stderr + err = reexecCommand.Start() + if err != nil { + err = fmt.Errorf("failed to start reexec process: %w", err) + return err } // Close child ends of sockets and pipes. @@ -447,7 +415,68 @@ func reexecUnikontainer(cmd *cli.Command) error { } }() - // execve + // terminal setup happens here, not in urunc create, so that nsenter + // failures are not swallowed by a prematurely attached PTY. by deferring + // this until after nsenter exits cleanly, errors propagate normally. + if unikontainer.Spec.Process.Terminal { + consoleSocket := cmd.String("console-socket") + if consoleSocket != "" { + ptm, pts, err := pty.Open() + if err != nil { + return fmt.Errorf("failed to open pty: %w", err) + } + defer ptm.Close() + defer pts.Close() + + conn, err := net.Dial("unix", consoleSocket) + if err != nil { + return fmt.Errorf("failed to dial console socket: %w", err) + } + defer conn.Close() + + uc, ok := conn.(*net.UnixConn) + if !ok { + return fmt.Errorf("failed to cast unix socket") + } + defer uc.Close() + + // send the master fd over the socket to containerd + oob := unix.UnixRights(int(ptm.Fd())) + _, _, err = uc.WriteMsgUnix([]byte(ptm.Name()), oob, nil) + if err != nil { + return fmt.Errorf("failed to send PTY file descriptor over socket: %w", err) + } + + // detach and establish the pty slave as the controlling terminal + // before we execve into the vmm payload + _, err = unix.Setsid() + if err != nil { + return fmt.Errorf("failed to setsid: %w", err) + } + + err = unix.IoctlSetInt(int(pts.Fd()), unix.TIOCSCTTY, 0) + if err != nil { + return fmt.Errorf("failed to set controlling terminal: %w", err) + } + + // map standard io to the pty slave + err = unix.Dup2(int(pts.Fd()), 0) + if err != nil { + return fmt.Errorf("failed to dup pty to stdin: %w", err) + } + + err = unix.Dup2(int(pts.Fd()), 1) + if err != nil { + return fmt.Errorf("failed to dup pty to stdout: %w", err) + } + + err = unix.Dup2(int(pts.Fd()), 2) + if err != nil { + return fmt.Errorf("failed to dup pty to stderr: %w", err) + } + } + } + // we need to pass metrics to Exec() function, as the unikontainer // struct does not have the part of urunc config that configures metrics var sockErr error