-
Notifications
You must be signed in to change notification settings - Fork 195
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
libpriv: Stop digging in private libcap internals #4769
Conversation
cargo is complaining about this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work debugging this.
{ | ||
/* Unfortunately, libcap doesn't expose any APIs to get the raw xattr value. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😢
When handling file caps, we need to be able to get the actual xattr bytes. Currently, libcap doesn't provide a public API for this. Meanwhile, we've been hacking around this by redefining internal libcap structs and using internal fields. This is extremely brittle since any change to the struct will break us. And this is exactly what happened in the libcap currently in rawhide. A new field was added to the struct: https://git.kernel.org/pub/scm/libs/libcap/libcap.git//commit/?id=aca076443591ba18438b60e41294b59a324daf04 This patch works around the lack of a public API a different way: we allocate an `O_TMPFILE`, set caps on it, then read back the xattr. This is more heavyweight but few binaries actually have file caps anyway. It also requires `CAP_SETFCAP`, but that's already the case today since the assembly code also applies them to checked out files on-disk. No new tests for this; we have existing coverage for file caps. That coverage would've caught this if we ran CI against rawhide. Closes coreos#4765
Woohoo! Thanks! |
{ | ||
/* Unfortunately, libcap doesn't expose any APIs to get the raw xattr value. | ||
* For now, we hackily dance around this by writting it out to a file and then | ||
* re-reading from it. */ | ||
cap_t caps = cap_from_text (fcap); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
scanner says:
leaked_storage: Variable "caps" going out of scope leaks the storage it points to.
Should we be doing something like free(caps) before returning null?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ouch, sorry. Fix in #4811.
When handling file caps, we need to be able to get the actual xattr
bytes. Currently, libcap doesn't provide a public API for this.
Meanwhile, we've been hacking around this by redefining internal libcap
structs and using internal fields. This is extremely brittle since any
change to the struct will break us.
And this is exactly what happened in the libcap currently in rawhide. A
new field was added to the struct:
https://git.kernel.org/pub/scm/libs/libcap/libcap.git//commit/?id=aca076443591ba18438b60e41294b59a324daf04
This patch works around the lack of a public API a different way: we
allocate an
O_TMPFILE
, set caps on it, then read back the xattr. Thisis more heavyweight but few binaries actually have file caps anyway. It
also requires
CAP_SETFCAP
, but that's already the case today since theassembly code also applies them to checked out files on-disk.
No new tests for this; we have existing coverage for file caps. That
coverage would've caught this if we ran CI against rawhide.
Closes #4765