Skip to content
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.

Sign up for GitHub

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

Jump to bottom

syscall: SysProcAttr.Noctty only works if child's stdin is controlling terminal #53601

Open
FiloSottile opened this issue Jun 29, 2022 · 5 comments
Open

syscall: SysProcAttr.Noctty only works if child's stdin is controlling terminal #53601

FiloSottile opened this issue Jun 29, 2022 · 5 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Backlog

Comments

@FiloSottile
Copy link
Contributor

SysProcAttr.Noctty is implemented by calling RawSyscall(SYS_IOCTL, 0, uintptr(TIOCNOTTY), 0) before exec. The TIOCNOTTY ioctl is only implemented on the file descriptor of the controlling terminal, which is not necessarily attached to the child's FD 0. This means that SysProcAttr.Noctty only works when cmd.Stdin is the session terminal, and it's not possible to run a child that is both without a terminal and with a functioning stdin.

This came up because I am writing tests for an application that uses /dev/tty to interact with the user, and stdin to take input separately. I want to test what happens when /dev/tty is not available, so I start the child with SysProcAttr.Noctty set, and a pipe for stdin. The result is that TIOCNOTTY is issued on the pipe, causing operation not supported by device.

Here's a short example.

package main

import (
	"os"
	"os/exec"
	"syscall"
)

func main() {
	if len(os.Args) > 1 && os.Args[1] == "child" {
		_, err := os.OpenFile("/dev/tty", os.O_RDWR, 0)
		if err != nil {
			println(err.Error()) // expect: "device not configured"
		}
		return
	}

	cmd := exec.Command(os.Args[0], "child")
	// Uncomment this block to have Noccty work correctly,
	// but making stdin in the child unusable.
	// f, err := os.OpenFile("/dev/tty", os.O_RDWR, 0)
	// if err != nil {
	// 	panic(err)
	// }
	// cmd.Stdin = f
	cmd.Stderr = os.Stderr // to see the output
	cmd.SysProcAttr = &syscall.SysProcAttr{}
	cmd.SysProcAttr.Noctty = true
	if err := cmd.Run(); err != nil {
		panic(err)
	}
}
@FiloSottile
Copy link
Contributor Author

Also, since Setctty is applied after Noctty, we also can't test the case in which stdin is a terminal, but there is no controlling terminal (which could have been possible if the sequence of operations was inverted).

@ianlancetaylor
Copy link
Contributor

Well, Noctty is acting as documented. Any suggestions for what to add here?

@FiloSottile
Copy link
Contributor Author

The most straightforward solution to me seems opening /dev/tty and issuing TIOCNOTTY on that, but maybe I don't see some reason it would be undesirable. It could still try FD 0 as a fallback if for some reason /dev/tty is not available. The behavior of any application that currently works would not change.

@ianlancetaylor
Copy link
Contributor

Sounds plausible. Will be interesting to see whether anything breaks.

@dr2chase dr2chase added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jun 30, 2022
@dr2chase
Copy link
Contributor

Is anyone here planning to propose a more detailed fix (e.g. a CL), or should I look elsewhere for people to bother? (I tagged it needs decision because I think testing the fix pretty much gives us the decision. And also milestone 1.20, most likely?)

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 7, 2022
@seankhliao seankhliao added this to the Go1.20 milestone Aug 20, 2022
@mknyszek mknyszek modified the milestones: Go1.20, Backlog Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
Go Compiler / Runtime
Status: Todo
Milestone
Backlog
Development

No branches or pull requests
6 participants
@FiloSottile @mknyszek @dr2chase @ianlancetaylor @gopherbot @seankhliao