From 90c8f94a055257f9ab343137cbada4e658750fbb Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Mon, 6 Feb 2023 11:48:46 -0500 Subject: [PATCH] unix: avoid converting non-pointers to unsafe.Pointer in PtraceIO Despite having the misleading type "void*" in the C API, the "offs" field of the ptrace_io_desc struct is an offset within the child process, and thus is not necessarily a valid pointer at all in the parent process. The Go unsafe.Pointer type must refer only to valid pointers, so converting this field through unsafe.Pointer is incorrect and (in some cases) dangerous. While we're here, let's also rename the "addr" function argument to "offs", since that's the corresponding ptrace_io_desc field. It's very confusing to have a function argument named "attr" that doesn't map to the struct field of the same name! For golang/go#58351. Change-Id: Id899f823e8d398b51fb0c42f466d7ae2f957c26b Reviewed-on: https://go-review.googlesource.com/c/sys/+/465675 Run-TryBot: Bryan Mills Auto-Submit: Bryan Mills TryBot-Result: Gopher Robot Reviewed-by: Ian Lance Taylor --- unix/syscall_freebsd_386.go | 9 +++++++-- unix/syscall_freebsd_amd64.go | 9 +++++++-- unix/syscall_freebsd_arm.go | 9 +++++++-- unix/syscall_freebsd_arm64.go | 9 +++++++-- unix/syscall_freebsd_riscv64.go | 9 +++++++-- 5 files changed, 35 insertions(+), 10 deletions(-) diff --git a/unix/syscall_freebsd_386.go b/unix/syscall_freebsd_386.go index b11ede89a..6a91d471d 100644 --- a/unix/syscall_freebsd_386.go +++ b/unix/syscall_freebsd_386.go @@ -60,8 +60,13 @@ func PtraceGetFsBase(pid int, fsbase *int64) (err error) { return ptrace(PT_GETFSBASE, pid, uintptr(unsafe.Pointer(fsbase)), 0) } -func PtraceIO(req int, pid int, addr uintptr, out []byte, countin int) (count int, err error) { - ioDesc := PtraceIoDesc{Op: int32(req), Offs: uintptr(unsafe.Pointer(addr)), Addr: uintptr(unsafe.Pointer(&out[0])), Len: uint32(countin)} +func PtraceIO(req int, pid int, offs uintptr, out []byte, countin int) (count int, err error) { + ioDesc := PtraceIoDesc{ + Op: int32(req), + Offs: offs, + Addr: uintptr(unsafe.Pointer(&out[0])), // TODO(#58351): this is not safe. + Len: uint32(countin), + } err = ptrace(PT_IO, pid, uintptr(unsafe.Pointer(&ioDesc)), 0) return int(ioDesc.Len), err } diff --git a/unix/syscall_freebsd_amd64.go b/unix/syscall_freebsd_amd64.go index 9ed8eec6c..48110a0ab 100644 --- a/unix/syscall_freebsd_amd64.go +++ b/unix/syscall_freebsd_amd64.go @@ -60,8 +60,13 @@ func PtraceGetFsBase(pid int, fsbase *int64) (err error) { return ptrace(PT_GETFSBASE, pid, uintptr(unsafe.Pointer(fsbase)), 0) } -func PtraceIO(req int, pid int, addr uintptr, out []byte, countin int) (count int, err error) { - ioDesc := PtraceIoDesc{Op: int32(req), Offs: uintptr(unsafe.Pointer(addr)), Addr: uintptr(unsafe.Pointer(&out[0])), Len: uint64(countin)} +func PtraceIO(req int, pid int, offs uintptr, out []byte, countin int) (count int, err error) { + ioDesc := PtraceIoDesc{ + Op: int32(req), + Offs: offs, + Addr: uintptr(unsafe.Pointer(&out[0])), // TODO(#58351): this is not safe. + Len: uint64(countin), + } err = ptrace(PT_IO, pid, uintptr(unsafe.Pointer(&ioDesc)), 0) return int(ioDesc.Len), err } diff --git a/unix/syscall_freebsd_arm.go b/unix/syscall_freebsd_arm.go index f8ac98247..52f1d4b75 100644 --- a/unix/syscall_freebsd_arm.go +++ b/unix/syscall_freebsd_arm.go @@ -56,8 +56,13 @@ func sendfile(outfd int, infd int, offset *int64, count int) (written int, err e func Syscall9(num, a1, a2, a3, a4, a5, a6, a7, a8, a9 uintptr) (r1, r2 uintptr, err syscall.Errno) -func PtraceIO(req int, pid int, addr uintptr, out []byte, countin int) (count int, err error) { - ioDesc := PtraceIoDesc{Op: int32(req), Offs: uintptr(unsafe.Pointer(addr)), Addr: uintptr(unsafe.Pointer(&out[0])), Len: uint32(countin)} +func PtraceIO(req int, pid int, offs uintptr, out []byte, countin int) (count int, err error) { + ioDesc := PtraceIoDesc{ + Op: int32(req), + Offs: offs, + Addr: uintptr(unsafe.Pointer(&out[0])), // TODO(#58351): this is not safe. + Len: uint32(countin), + } err = ptrace(PT_IO, pid, uintptr(unsafe.Pointer(&ioDesc)), 0) return int(ioDesc.Len), err } diff --git a/unix/syscall_freebsd_arm64.go b/unix/syscall_freebsd_arm64.go index 8e932036e..5537ee4f2 100644 --- a/unix/syscall_freebsd_arm64.go +++ b/unix/syscall_freebsd_arm64.go @@ -56,8 +56,13 @@ func sendfile(outfd int, infd int, offset *int64, count int) (written int, err e func Syscall9(num, a1, a2, a3, a4, a5, a6, a7, a8, a9 uintptr) (r1, r2 uintptr, err syscall.Errno) -func PtraceIO(req int, pid int, addr uintptr, out []byte, countin int) (count int, err error) { - ioDesc := PtraceIoDesc{Op: int32(req), Offs: uintptr(unsafe.Pointer(addr)), Addr: uintptr(unsafe.Pointer(&out[0])), Len: uint64(countin)} +func PtraceIO(req int, pid int, offs uintptr, out []byte, countin int) (count int, err error) { + ioDesc := PtraceIoDesc{ + Op: int32(req), + Offs: offs, + Addr: uintptr(unsafe.Pointer(&out[0])), // TODO(#58351): this is not safe. + Len: uint64(countin), + } err = ptrace(PT_IO, pid, uintptr(unsafe.Pointer(&ioDesc)), 0) return int(ioDesc.Len), err } diff --git a/unix/syscall_freebsd_riscv64.go b/unix/syscall_freebsd_riscv64.go index cbe122278..164abd5d2 100644 --- a/unix/syscall_freebsd_riscv64.go +++ b/unix/syscall_freebsd_riscv64.go @@ -56,8 +56,13 @@ func sendfile(outfd int, infd int, offset *int64, count int) (written int, err e func Syscall9(num, a1, a2, a3, a4, a5, a6, a7, a8, a9 uintptr) (r1, r2 uintptr, err syscall.Errno) -func PtraceIO(req int, pid int, addr uintptr, out []byte, countin int) (count int, err error) { - ioDesc := PtraceIoDesc{Op: int32(req), Offs: uintptr(unsafe.Pointer(addr)), Addr: uintptr(unsafe.Pointer(&out[0])), Len: uint64(countin)} +func PtraceIO(req int, pid int, offs uintptr, out []byte, countin int) (count int, err error) { + ioDesc := PtraceIoDesc{ + Op: int32(req), + Offs: offs, + Addr: uintptr(unsafe.Pointer(&out[0])), // TODO(#58351): this is not safe. + Len: uint64(countin), + } err = ptrace(PT_IO, pid, uintptr(unsafe.Pointer(&ioDesc)), 0) return int(ioDesc.Len), err }