-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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: FreeBSD PtraceIO violates unsafe.Pointer rules #58351
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
bcmills
added
OS-FreeBSD
NeedsInvestigation
Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
labels
Feb 6, 2023
gopherbot
added
the
compiler/runtime
Issues related to the Go compiler and/or runtime.
label
Feb 6, 2023
Change https://go.dev/cl/465235 mentions this issue: |
Change https://go.dev/cl/465675 mentions this issue: |
Change https://go.dev/cl/465676 mentions this issue: |
gopherbot
pushed a commit
to golang/sys
that referenced
this issue
Feb 7, 2023
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 <[email protected]> Auto-Submit: Bryan Mills <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>
aarzilli
added a commit
to aarzilli/delve
that referenced
this issue
Feb 14, 2023
Per https://pkg.go.dev/unsafe#Pointer conversions from unsafe.Pointer to uintptr are only safe in limited circumstances. In particular only conversions made in the syscall call are pinned. Additionally add a call to runtime.KeepAlive to mitigate the bug described in: golang/go#58351
derekparker
pushed a commit
to go-delve/delve
that referenced
this issue
Feb 14, 2023
Per https://pkg.go.dev/unsafe#Pointer conversions from unsafe.Pointer to uintptr are only safe in limited circumstances. In particular only conversions made in the syscall call are pinned. Additionally add a call to runtime.KeepAlive to mitigate the bug described in: golang/go#58351
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
The fix for #54113 appears to have been only partial.
GOOS=freebsd GOARCH=amd64 go vet ./...
in thex/sys
module shows a true-positive violation of theunsafe.Pointer
rules insyscall_freebsd_amd64.go
:Absent an API for pinning Go object addresses (#46787), this is incorrect and could lead to arbitrary memory corruption: if the
out
slice refers to memory in the Go heap, that memory can be collected (and reused for another allocation) concurrently with the call toptrace
.(Note that the
unsafe.Pointer
rule that allows conversion of anunsafe.Pointer
to auintptr
when callingsyscall.Syscall
applies only with the call expression itself, not within other variable declarations in the same function that makes the call.This was apparently masked by #41205.
(attn @golang/freebsd; CC @tklauser @ianlancetaylor @aarzilli @mdempsky)
The text was updated successfully, but these errors were encountered: