x/sys/unix: xattr functions erroneously pass Go pointers as type uintptr on BSDs

[bcmills]

The unsafe.Pointer rules allow “conversion of a Pointer to a uintptr when calling syscall.Syscall”, with a caveat:

If a pointer argument must be converted to uintptr for use as an argument, that conversion must appear in the call expression itself:
…
The compiler handles a Pointer converted to a uintptr in the argument list of a call to a function implemented in assembly by arranging that the referenced allocated object, if any, is retained and not moved until the call completes, even though from the types alone it would appear that the object is no longer needed during the call.

The linux and darwin implementations of the .*xattr functions (such as Getxattr) appear to comply with that requirement.

However, the BSD implementation does not:
https://cs.opensource.google/go/x/sys/+/master:unix/xattr_bsd.go;l=60;drc=4e121b1efb52d0ccc0c89c55272b7c3da9a475f8

Instead of calling syscall.Syscall directly, it passes the dest pointer to another helper function (such as ExtattrGetFile) as type uintptr. That can cause a use-after-free bug if the Go garbage collector reclaims or relocates the destination slice concurrently with the system call. (The danger is especially high if the caller happens not to refer to the destination buffer after the call, although that situation is less likely.)

It appears that the erroneous signatures were added in CL 147850043.

Probably this will need to be fixed by adding (unexported?) variants of the Extattr functions, with the data arguments correctly encoded as type *byte or unsafe.Pointer instead of uintptr, and then switching Getxattr and similar to use those safer variants. (For comparison, see the ioctl / ioctlPtr split on Linux from #44834.)

(attn @golang/freebsd @golang/netbsd; CC @tklauser @bradfitz @ianlancetaylor)

[bcmills]

(Found while investigating #41205.)

[ayang64]

@bcmills -- does this mean we should update mksyscall.go in x/sys/unix?

[gopherbot]

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

[bcmills]

does this mean we should update mksyscall.go in x/sys/unix?

Since we don't need to change the platform calling conventions, I don't think a change should be needed in mksyscall.go. Instead, we need to fix the signatures in the //sys declarations in syscall_freebsd.go and syscall_netbsd.go to correctly match the C signatures for those system calls.

[jharshman]

So these declaration for *xtattr* calls found at the following lines need to be updated correct?

https://go.googlesource.com/sys/+/refs/heads/master/unix/syscall_freebsd.go#360
https://go.googlesource.com/sys/+/refs/heads/master/unix/syscall_netbsd.go#267

EDIT: Didn't see the date of the last comment when cruising for open issues. I see that it has been assigned.