From 00d8004a14487f8c7b7fdfe44b95e9f6c4590f5f Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Wed, 15 Mar 2023 17:52:25 +0100 Subject: [PATCH] unix: match ioctl req argument type to libc type On Solaris, AIX, and zOS, the req argument of ioctl() is a signed int, not an unsigned long like on other platforms, which means many constants are negative, causing friction when passing them to a uint argument. Correct the signature of these functions to pass the req argument as signed, just like libc. Fixes golang/go#59030. Change-Id: Ia14e92a150f4b5fb9488c5032ca296cb786e9811 Reviewed-on: https://go-review.googlesource.com/c/sys/+/476515 Reviewed-by: Ian Lance Taylor Reviewed-by: Bryan Mills Run-TryBot: Jason Donenfeld TryBot-Result: Gopher Robot Reviewed-by: Nahum Shalman --- unix/ioctl_signed.go | 70 ++++++++++++++++++++++++++++ unix/{ioctl.go => ioctl_unsigned.go} | 4 +- unix/ioctl_zos.go | 12 ++--- unix/syscall_aix.go | 4 +- unix/syscall_solaris.go | 20 ++++---- unix/syscall_solaris_test.go | 6 +-- unix/syscall_zos_s390x.go | 4 +- unix/zsyscall_aix_ppc.go | 4 +- unix/zsyscall_aix_ppc64.go | 8 ++-- unix/zsyscall_solaris_amd64.go | 4 +- unix/zsyscall_zos_s390x.go | 4 +- 11 files changed, 103 insertions(+), 37 deletions(-) create mode 100644 unix/ioctl_signed.go rename unix/{ioctl.go => ioctl_unsigned.go} (92%) diff --git a/unix/ioctl_signed.go b/unix/ioctl_signed.go new file mode 100644 index 000000000..7def9580e --- /dev/null +++ b/unix/ioctl_signed.go @@ -0,0 +1,70 @@ +// Copyright 2018 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build aix || solaris +// +build aix solaris + +package unix + +import ( + "unsafe" +) + +// ioctl itself should not be exposed directly, but additional get/set +// functions for specific types are permissible. + +// IoctlSetInt performs an ioctl operation which sets an integer value +// on fd, using the specified request number. +func IoctlSetInt(fd int, req int, value int) error { + return ioctl(fd, req, uintptr(value)) +} + +// IoctlSetPointerInt performs an ioctl operation which sets an +// integer value on fd, using the specified request number. The ioctl +// argument is called with a pointer to the integer value, rather than +// passing the integer value directly. +func IoctlSetPointerInt(fd int, req int, value int) error { + v := int32(value) + return ioctlPtr(fd, req, unsafe.Pointer(&v)) +} + +// IoctlSetWinsize performs an ioctl on fd with a *Winsize argument. +// +// To change fd's window size, the req argument should be TIOCSWINSZ. +func IoctlSetWinsize(fd int, req int, value *Winsize) error { + // TODO: if we get the chance, remove the req parameter and + // hardcode TIOCSWINSZ. + return ioctlPtr(fd, req, unsafe.Pointer(value)) +} + +// IoctlSetTermios performs an ioctl on fd with a *Termios. +// +// The req value will usually be TCSETA or TIOCSETA. +func IoctlSetTermios(fd int, req int, value *Termios) error { + // TODO: if we get the chance, remove the req parameter. + return ioctlPtr(fd, req, unsafe.Pointer(value)) +} + +// IoctlGetInt performs an ioctl operation which gets an integer value +// from fd, using the specified request number. +// +// A few ioctl requests use the return value as an output parameter; +// for those, IoctlRetInt should be used instead of this function. +func IoctlGetInt(fd int, req int) (int, error) { + var value int + err := ioctlPtr(fd, req, unsafe.Pointer(&value)) + return value, err +} + +func IoctlGetWinsize(fd int, req int) (*Winsize, error) { + var value Winsize + err := ioctlPtr(fd, req, unsafe.Pointer(&value)) + return &value, err +} + +func IoctlGetTermios(fd int, req int) (*Termios, error) { + var value Termios + err := ioctlPtr(fd, req, unsafe.Pointer(&value)) + return &value, err +} diff --git a/unix/ioctl.go b/unix/ioctl_unsigned.go similarity index 92% rename from unix/ioctl.go rename to unix/ioctl_unsigned.go index 7ce8dd406..649913d1e 100644 --- a/unix/ioctl.go +++ b/unix/ioctl_unsigned.go @@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -//go:build aix || darwin || dragonfly || freebsd || hurd || linux || netbsd || openbsd || solaris -// +build aix darwin dragonfly freebsd hurd linux netbsd openbsd solaris +//go:build darwin || dragonfly || freebsd || hurd || linux || netbsd || openbsd +// +build darwin dragonfly freebsd hurd linux netbsd openbsd package unix diff --git a/unix/ioctl_zos.go b/unix/ioctl_zos.go index 6532f09af..cdc21bf76 100644 --- a/unix/ioctl_zos.go +++ b/unix/ioctl_zos.go @@ -17,14 +17,14 @@ import ( // IoctlSetInt performs an ioctl operation which sets an integer value // on fd, using the specified request number. -func IoctlSetInt(fd int, req uint, value int) error { +func IoctlSetInt(fd int, req int, value int) error { return ioctl(fd, req, uintptr(value)) } // IoctlSetWinsize performs an ioctl on fd with a *Winsize argument. // // To change fd's window size, the req argument should be TIOCSWINSZ. -func IoctlSetWinsize(fd int, req uint, value *Winsize) error { +func IoctlSetWinsize(fd int, req int, value *Winsize) error { // TODO: if we get the chance, remove the req parameter and // hardcode TIOCSWINSZ. return ioctlPtr(fd, req, unsafe.Pointer(value)) @@ -33,7 +33,7 @@ func IoctlSetWinsize(fd int, req uint, value *Winsize) error { // IoctlSetTermios performs an ioctl on fd with a *Termios. // // The req value is expected to be TCSETS, TCSETSW, or TCSETSF -func IoctlSetTermios(fd int, req uint, value *Termios) error { +func IoctlSetTermios(fd int, req int, value *Termios) error { if (req != TCSETS) && (req != TCSETSW) && (req != TCSETSF) { return ENOSYS } @@ -47,13 +47,13 @@ func IoctlSetTermios(fd int, req uint, value *Termios) error { // // A few ioctl requests use the return value as an output parameter; // for those, IoctlRetInt should be used instead of this function. -func IoctlGetInt(fd int, req uint) (int, error) { +func IoctlGetInt(fd int, req int) (int, error) { var value int err := ioctlPtr(fd, req, unsafe.Pointer(&value)) return value, err } -func IoctlGetWinsize(fd int, req uint) (*Winsize, error) { +func IoctlGetWinsize(fd int, req int) (*Winsize, error) { var value Winsize err := ioctlPtr(fd, req, unsafe.Pointer(&value)) return &value, err @@ -62,7 +62,7 @@ func IoctlGetWinsize(fd int, req uint) (*Winsize, error) { // IoctlGetTermios performs an ioctl on fd with a *Termios. // // The req value is expected to be TCGETS -func IoctlGetTermios(fd int, req uint) (*Termios, error) { +func IoctlGetTermios(fd int, req int) (*Termios, error) { var value Termios if req != TCGETS { return &value, ENOSYS diff --git a/unix/syscall_aix.go b/unix/syscall_aix.go index d9f5544cc..c406ae00f 100644 --- a/unix/syscall_aix.go +++ b/unix/syscall_aix.go @@ -408,8 +408,8 @@ func (w WaitStatus) CoreDump() bool { return w&0x80 == 0x80 } func (w WaitStatus) TrapCause() int { return -1 } -//sys ioctl(fd int, req uint, arg uintptr) (err error) -//sys ioctlPtr(fd int, req uint, arg unsafe.Pointer) (err error) = ioctl +//sys ioctl(fd int, req int, arg uintptr) (err error) +//sys ioctlPtr(fd int, req int, arg unsafe.Pointer) (err error) = ioctl // fcntl must never be called with cmd=F_DUP2FD because it doesn't work on AIX // There is no way to create a custom fcntl and to keep //sys fcntl easily, diff --git a/unix/syscall_solaris.go b/unix/syscall_solaris.go index 3120d44d5..b600a289d 100644 --- a/unix/syscall_solaris.go +++ b/unix/syscall_solaris.go @@ -545,24 +545,24 @@ func Minor(dev uint64) uint32 { * Expose the ioctl function */ -//sys ioctlRet(fd int, req uint, arg uintptr) (ret int, err error) = libc.ioctl -//sys ioctlPtrRet(fd int, req uint, arg unsafe.Pointer) (ret int, err error) = libc.ioctl +//sys ioctlRet(fd int, req int, arg uintptr) (ret int, err error) = libc.ioctl +//sys ioctlPtrRet(fd int, req int, arg unsafe.Pointer) (ret int, err error) = libc.ioctl -func ioctl(fd int, req uint, arg uintptr) (err error) { +func ioctl(fd int, req int, arg uintptr) (err error) { _, err = ioctlRet(fd, req, arg) return err } -func ioctlPtr(fd int, req uint, arg unsafe.Pointer) (err error) { +func ioctlPtr(fd int, req int, arg unsafe.Pointer) (err error) { _, err = ioctlPtrRet(fd, req, arg) return err } -func IoctlSetTermio(fd int, req uint, value *Termio) error { +func IoctlSetTermio(fd int, req int, value *Termio) error { return ioctlPtr(fd, req, unsafe.Pointer(value)) } -func IoctlGetTermio(fd int, req uint) (*Termio, error) { +func IoctlGetTermio(fd int, req int) (*Termio, error) { var value Termio err := ioctlPtr(fd, req, unsafe.Pointer(&value)) return &value, err @@ -1079,11 +1079,11 @@ func Getmsg(fd int, cl []byte, data []byte) (retCl []byte, retData []byte, flags return retCl, retData, flags, nil } -func IoctlSetIntRetInt(fd int, req uint, arg int) (int, error) { +func IoctlSetIntRetInt(fd int, req int, arg int) (int, error) { return ioctlRet(fd, req, uintptr(arg)) } -func IoctlSetString(fd int, req uint, val string) error { +func IoctlSetString(fd int, req int, val string) error { bs := make([]byte, len(val)+1) copy(bs[:len(bs)-1], val) err := ioctlPtr(fd, req, unsafe.Pointer(&bs[0])) @@ -1119,7 +1119,7 @@ func (l *Lifreq) GetLifruUint() uint { return *(*uint)(unsafe.Pointer(&l.Lifru[0])) } -func IoctlLifreq(fd int, req uint, l *Lifreq) error { +func IoctlLifreq(fd int, req int, l *Lifreq) error { return ioctlPtr(fd, req, unsafe.Pointer(l)) } @@ -1130,6 +1130,6 @@ func (s *Strioctl) SetInt(i int) { s.Dp = (*int8)(unsafe.Pointer(&i)) } -func IoctlSetStrioctlRetInt(fd int, req uint, s *Strioctl) (int, error) { +func IoctlSetStrioctlRetInt(fd int, req int, s *Strioctl) (int, error) { return ioctlPtrRet(fd, req, unsafe.Pointer(s)) } diff --git a/unix/syscall_solaris_test.go b/unix/syscall_solaris_test.go index 88eccaf09..773c28ef7 100644 --- a/unix/syscall_solaris_test.go +++ b/unix/syscall_solaris_test.go @@ -405,17 +405,13 @@ func TestLifreqGetMTU(t *testing.T) { if err != nil { t.Fatalf("could not open udp socket: %v", err) } - // SIOCGLIFMTU is negative which confuses the compiler if used inline: - // Using "unix.IoctlLifreq(ip_fd, unix.SIOCGLIFMTU, &l)" results in - // "constant -1065850502 overflows uint" - reqnum := int(unix.SIOCGLIFMTU) var l unix.Lifreq for link, mtu := range tc { err = l.SetName(link) if err != nil { t.Fatalf("Lifreq.SetName(%q) failed: %v", link, err) } - if err = unix.IoctlLifreq(ip_fd, uint(reqnum), &l); err != nil { + if err = unix.IoctlLifreq(ip_fd, unix.SIOCGLIFMTU, &l); err != nil { t.Fatalf("unable to SIOCGLIFMTU: %v", err) } m := l.GetLifruUint() diff --git a/unix/syscall_zos_s390x.go b/unix/syscall_zos_s390x.go index b295497ae..d3d49ec3e 100644 --- a/unix/syscall_zos_s390x.go +++ b/unix/syscall_zos_s390x.go @@ -212,8 +212,8 @@ func (cmsg *Cmsghdr) SetLen(length int) { //sys sendmsg(s int, msg *Msghdr, flags int) (n int, err error) = SYS___SENDMSG_A //sys mmap(addr uintptr, length uintptr, prot int, flag int, fd int, pos int64) (ret uintptr, err error) = SYS_MMAP //sys munmap(addr uintptr, length uintptr) (err error) = SYS_MUNMAP -//sys ioctl(fd int, req uint, arg uintptr) (err error) = SYS_IOCTL -//sys ioctlPtr(fd int, req uint, arg unsafe.Pointer) (err error) = SYS_IOCTL +//sys ioctl(fd int, req int, arg uintptr) (err error) = SYS_IOCTL +//sys ioctlPtr(fd int, req int, arg unsafe.Pointer) (err error) = SYS_IOCTL //sys Access(path string, mode uint32) (err error) = SYS___ACCESS_A //sys Chdir(path string) (err error) = SYS___CHDIR_A diff --git a/unix/zsyscall_aix_ppc.go b/unix/zsyscall_aix_ppc.go index 0a9984eab..9a257219d 100644 --- a/unix/zsyscall_aix_ppc.go +++ b/unix/zsyscall_aix_ppc.go @@ -212,7 +212,7 @@ func wait4(pid Pid_t, status *_C_int, options int, rusage *Rusage) (wpid Pid_t, // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT -func ioctl(fd int, req uint, arg uintptr) (err error) { +func ioctl(fd int, req int, arg uintptr) (err error) { r0, er := C.ioctl(C.int(fd), C.int(req), C.uintptr_t(arg)) if r0 == -1 && er != nil { err = er @@ -222,7 +222,7 @@ func ioctl(fd int, req uint, arg uintptr) (err error) { // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT -func ioctlPtr(fd int, req uint, arg unsafe.Pointer) (err error) { +func ioctlPtr(fd int, req int, arg unsafe.Pointer) (err error) { r0, er := C.ioctl(C.int(fd), C.int(req), C.uintptr_t(uintptr(arg))) if r0 == -1 && er != nil { err = er diff --git a/unix/zsyscall_aix_ppc64.go b/unix/zsyscall_aix_ppc64.go index c3783bfaa..6de80c20c 100644 --- a/unix/zsyscall_aix_ppc64.go +++ b/unix/zsyscall_aix_ppc64.go @@ -93,8 +93,8 @@ func wait4(pid Pid_t, status *_C_int, options int, rusage *Rusage) (wpid Pid_t, // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT -func ioctl(fd int, req uint, arg uintptr) (err error) { - _, e1 := callioctl(fd, int(req), arg) +func ioctl(fd int, req int, arg uintptr) (err error) { + _, e1 := callioctl(fd, req, arg) if e1 != 0 { err = errnoErr(e1) } @@ -103,8 +103,8 @@ func ioctl(fd int, req uint, arg uintptr) (err error) { // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT -func ioctlPtr(fd int, req uint, arg unsafe.Pointer) (err error) { - _, e1 := callioctl_ptr(fd, int(req), arg) +func ioctlPtr(fd int, req int, arg unsafe.Pointer) (err error) { + _, e1 := callioctl_ptr(fd, req, arg) if e1 != 0 { err = errnoErr(e1) } diff --git a/unix/zsyscall_solaris_amd64.go b/unix/zsyscall_solaris_amd64.go index 305162fb3..609d1c598 100644 --- a/unix/zsyscall_solaris_amd64.go +++ b/unix/zsyscall_solaris_amd64.go @@ -643,7 +643,7 @@ func __minor(version int, dev uint64) (val uint) { // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT -func ioctlRet(fd int, req uint, arg uintptr) (ret int, err error) { +func ioctlRet(fd int, req int, arg uintptr) (ret int, err error) { r0, _, e1 := sysvicall6(uintptr(unsafe.Pointer(&procioctl)), 3, uintptr(fd), uintptr(req), uintptr(arg), 0, 0, 0) ret = int(r0) if e1 != 0 { @@ -654,7 +654,7 @@ func ioctlRet(fd int, req uint, arg uintptr) (ret int, err error) { // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT -func ioctlPtrRet(fd int, req uint, arg unsafe.Pointer) (ret int, err error) { +func ioctlPtrRet(fd int, req int, arg unsafe.Pointer) (ret int, err error) { r0, _, e1 := sysvicall6(uintptr(unsafe.Pointer(&procioctl)), 3, uintptr(fd), uintptr(req), uintptr(arg), 0, 0, 0) ret = int(r0) if e1 != 0 { diff --git a/unix/zsyscall_zos_s390x.go b/unix/zsyscall_zos_s390x.go index 07bfe2ef9..c31681743 100644 --- a/unix/zsyscall_zos_s390x.go +++ b/unix/zsyscall_zos_s390x.go @@ -257,7 +257,7 @@ func munmap(addr uintptr, length uintptr) (err error) { // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT -func ioctl(fd int, req uint, arg uintptr) (err error) { +func ioctl(fd int, req int, arg uintptr) (err error) { _, _, e1 := syscall_syscall(SYS_IOCTL, uintptr(fd), uintptr(req), uintptr(arg)) if e1 != 0 { err = errnoErr(e1) @@ -267,7 +267,7 @@ func ioctl(fd int, req uint, arg uintptr) (err error) { // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT -func ioctlPtr(fd int, req uint, arg unsafe.Pointer) (err error) { +func ioctlPtr(fd int, req int, arg unsafe.Pointer) (err error) { _, _, e1 := syscall_syscall(SYS_IOCTL, uintptr(fd), uintptr(req), uintptr(arg)) if e1 != 0 { err = errnoErr(e1)