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.

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

x/sys/unix: IoctlGetPtmget fail to get ptsname on NetBSD #66871

Open
iyzyi opened this issue Apr 17, 2024 · 4 comments · May be fixed by golang/sys#192
Open

x/sys/unix: IoctlGetPtmget fail to get ptsname on NetBSD #66871

iyzyi opened this issue Apr 17, 2024 · 4 comments · May be fixed by golang/sys#192
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. OS-NetBSD
Milestone

Comments

@iyzyi
Copy link

iyzyi commented Apr 17, 2024

Go version

go version go1.21.8 netbsd/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/root/.cache/go-build'
GOENV='/root/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='netbsd'
GOINSECURE=''
GOMODCACHE='/root/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='netbsd'
GOPATH='/root/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/pkg/go121'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/pkg/go121/pkg/tool/netbsd_amd64'
GOVCS=''
GOVERSION='go1.21.8'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build1420044071=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Although the test case TestIoctlPtmget passes on NetBSD, debugging revealed that ptm.Sn is a byte array filled with zeros, thus preventing us from obtaining the ptsname. The log consistently shows sfd = 0, ptsname = , whereas under normal circumstances, we expect something like sfd = 0, ptsname = /dev/pts/0 to be displayed.

Below is an example of relevant code for the amd64 architecture.

func TestIoctlPtmget(t *testing.T) {
	fd, err := unix.Open("/dev/ptmx", unix.O_NOCTTY|unix.O_RDWR, 0666)
	if err != nil {
		t.Skip("failed to open /dev/ptmx, skipping test")
	}
	defer unix.Close(fd)

	ptm, err := unix.IoctlGetPtmget(fd, unix.TIOCPTSNAME)
	if err != nil {
		t.Fatalf("IoctlGetPtmget: %v\n", err)
	}

	t.Logf("sfd = %v, ptsname = %v", ptm.Sfd, unix.ByteSliceToString(ptm.Sn[:]))
}

const TIOCPTSNAME = 0x40287448

type Ptmget struct {
	Cfd int32
	Sfd int32
	Cn  [1024]byte
	Sn  [1024]byte
}

According to my investigation, this error is caused by the mismatch in the lengths of the arrays Cn and Sn within the Ptmget structure with the value of TIOCPTSNAME.

In NetBSD: src/sys/sys/ttycom.h, we can find that the value of TIOCPTSNAME is related to the size of Ptmget.

#include <iostream>

#define	IOCPARM_MASK	0x1fff		/* parameter length, at most 13 bits */
#define	IOCPARM_SHIFT	16
#define	IOCGROUP_SHIFT	8
#define	_IOC(inout, group, num, len) \
    ((inout) | (((len) & IOCPARM_MASK) << IOCPARM_SHIFT) | \
    ((group) << IOCGROUP_SHIFT) | (num))
#define	IOC_OUT		(unsigned long)0x40000000
#define	_IOR(g,n,t)	_IOC(IOC_OUT,	(g), (n), sizeof(t))
#define TIOCPTSNAME 	 _IOR('t', 72, struct ptmget)	/* ptsname(3) */

#define PATH_MAX 1024

struct ptmget {
	int		cfd;
	int		sfd;
	char	cn[PATH_MAX];
	char	sn[PATH_MAX];
};

int main() {
    std::cout << TIOCPTSNAME;
}

When we set PATH_MAX to the currently used 1024 in the code, the corresponding value for TIOCPTSNAME should be 0x48087448, not the 0x40287448 defined for 386/amd64/arm64 (the arm definition matches correctly).

And when TIOCPTSNAME is 0x40287448 as used in the current code, the corresponding PATH_MAX should be 16. This indeed aligns with the description in the netbsd man page / ptm.

I will submit a CL to fix this error shortly. This is my first time submitting a CL for Go. Please let me know if there's anything inappropriate or if you have any feedback.

What did you see happen?

None

What did you expect to see?

None

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Apr 17, 2024
@gopherbot gopherbot added this to the Unreleased milestone Apr 17, 2024
@gopherbot
Copy link

Change https://go.dev/cl/579476 mentions this issue: unix: fix IoctlGetPtmget get empty ptsname on NetBSD.

@ianlancetaylor
Copy link
Contributor

CC @golang/netbsd

If I understand the bug report, it sounds like the fix should be to adjust the value of TIOCPTSNAME, not to change the struct size.

@iyzyi
Copy link
Author

iyzyi commented Apr 17, 2024

CC @golang/netbsd

If I understand the bug report, it sounds like the fix should be to adjust the value of TIOCPTSNAME, not to change the struct size.

Yes, IoctlGetPtmget will also get the correct ptsname by setting TIOCPTSNAME from 0x40287448 to 0x48087448 and not to change the Ptmget struct size.
May I ask if I need to follow this logic to modify the code I submitted?

@iyzyi iyzyi closed this as completed Apr 17, 2024
@iyzyi iyzyi reopened this Apr 17, 2024
@cherrymui cherrymui added NeedsFix The path to resolution is known, but the work has not been done. OS-NetBSD labels Apr 17, 2024
@ianlancetaylor
Copy link
Contributor

May I ask if I need to follow this logic to modify the code I submitted?

I'm sorry, I don't understand the question.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. OS-NetBSD
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

4 participants