-
Notifications
You must be signed in to change notification settings - Fork 18k
x/sys/unix: Capget/Capset writes past struct's memory #44312
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
Comments
Have you considered using the cap package instead? |
Hey Andrew, thanks for the suggestion! In my use case I'm doing the following: go func() {
runtime.LockOSThread() // exclusively bind the goroutine to a single os thread.
unix.Capget/Capset
unix.Setns // change netns of the os thread
exec.Start/Wait/Run/etc
// exit goroutine without unlocking to kill the os thread
} () |
Take a look at the |
Thanks for the pointer! Are there any plans to expose cap API with |
You should feel free to make such a feature request. The [There seems to be a lot of confusion about the importance of POSIX semantics for privilege mechanisms in general though. I wrote this to capture how trivial it is for unprivileged threads to trick the other threads into doing privileged things, for C/C++ but it applies to Go/CGo too.] |
Also, I forgot to mention, the |
Thanks! done: https://bugzilla.kernel.org/show_bug.cgi?id=211919
But then I'm back to square one using naked syscalls instead of nice libcap API. |
@ghost I think you need to open a PR against x/sys/unix, adding documentation for these functions (and/or usage example). |
I have a go program that is using unix.Capget/Capset and was randomly crashing.
Thanks to @ianlancetaylor this code was identified as a culprit:
Turns out that version 2 and version 3 linux capabilities are wider than version 1 capabilities.
Version 1 uses 32bit fields and fit perfectly into a single
unix.CapUserData
:However version 2 and version 3 capabilities are 64 bit and do not fit into a single
unix.CapUserData
.Instead of using a struct with wider fields however linux capget syscall uses the same struct but writes into 2 instances of the struct. It writes lower bits of the capabilities into the first struct and the higher bits into the second one.
So, confusingly the correct way of writing the code above would be:
Which is normal practice in C, but doesn't look like a typical go to me.
This code would write lower bits into
data[0]
and higher bits intodata[1]
. Then the client would have to be careful when setting capabilities, choosingdata[0]
vsdata[1]
depending on the capability, for example:Same applies to unix.Capset, except it would read past the first struct.
I'm wondering if the signature of
unix.Capget/Capset
should be changed into something like:to prevent issues like this. Such a signature would clearly indicate the amount of memory affected by the syscall.
Or maybe make the fields wider and repack during the call to account for a weird memory layout? That wouldn't work very well though if linux keeps expanding capabilities and would go even wider.
Thoughts?
The text was updated successfully, but these errors were encountered: