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: on FreeBSD PtraceLwpInfoStruct, PtraceIoDesc and PtraceIO all can not be used safely #54113

Closed
aarzilli opened this issue Jul 28, 2022 · 5 comments
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-FreeBSD
Milestone

Comments

@aarzilli
Copy link
Contributor

These all represent tracee addresses as *byte which makes -d=checkptr fail and the GC occasionally barf.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 28, 2022
@gopherbot gopherbot added this to the Unreleased milestone Jul 28, 2022
@cherrymui
Copy link
Member

cc @ianlancetaylor @bradfitz @tklauser @golang/runtime

@cherrymui cherrymui added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 28, 2022
@bcmills
Copy link
Contributor

bcmills commented Jul 29, 2022

(CC @golang/freebsd)

@gopherbot
Copy link

Change https://go.dev/cl/419915 mentions this issue: x/sys/unix: use uintptr for tracee addresses on FreeBSD

@hyangah
Copy link
Contributor

hyangah commented Sep 6, 2022

Based on the new port policy, cl/419915 needs reviews from @golang/freebsd

@gopherbot
Copy link

Change https://go.dev/cl/465676 mentions this issue: unix: fix a use-after-free bug in PtraceIO on freebsd

gopherbot pushed a commit to golang/sys that referenced this issue Feb 8, 2023
In CL 419915, both pointer fields of the PtraceIoDesc struct were
converted to type uintptr to address golang/go#54113.

However, that change was overzealous: the fix needed to convert fields
that refer to addresses in the child process, but the Addr field of
PtraceIoDesc is not even in the child process! It is instead an
address in the parent (Go) process.

Go's unsafe.Pointer rules prohibit converting a Go pointer to a
uintptr except when immediately converting back to an unsafe.Pointer
or calling a system call. Populating a PtraceIoDesc struct is neither
of those things, so converting the Addr field to uintptr introduced a
use-after-free bug.

This change reverts the change to the Addr field from CL 419915 and
consolidates the implementation of PtraceIO to reduce the the amount
of code that varies with GOARCH.

This change does not address the remaining ptrace uintptr bug
(golang/go#58387), which is also present in the Linux implementation.

Fixes golang/go#58351.
Updates golang/go#54113.
For golang/go#41205.

Change-Id: I14bdb4af42130aa7b4375e3f53fd1a0435f14307
Reviewed-on: https://go-review.googlesource.com/c/sys/+/465676
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Feb 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-FreeBSD
Projects
None yet
Development

No branches or pull requests

8 participants