[GIT PULL] capabilities
Linus Torvalds
torvalds at linux-foundation.org
Wed Nov 27 17:30:14 UTC 2024
On Mon, 18 Nov 2024 at 07:26, <sergeh at kernel.org> wrote:
>
> 2. Add a trace event for cap_capable (Jordan Rome).
So I've finally gotten around to this, but I absolutely detest how
this was written.
It was oddly written before, but now it's absolutely illegible. All
just to have one single tracepoint.
And it's all *stupid*.
The "capable_ns" thing is entirely pointless.
Why? It always has exactly one value: 'cred->user_ns'. Lookie here,
it's assigned exactly twice:
if (ns == cred->user_ns) {
if (cap_raised(cred->cap_effective, cap)) {
capable_ns = ns;
...
if ((ns->parent == cred->user_ns) && uid_eq(ns->owner,
cred->euid)) {
capable_ns = ns->parent;
and *both* times it's assigned something that we just checked is equal
to cred->user_ns.
And for this useless value, the already odd for-loop was written to be
even more odd, and the code added a new variable 'capable_ns'.
So I pulled this, tried to figure out _why_ it was written that oddly,
decided that the "why" was "because it's being stupid", and I unpulled
it again.
If we really need that trace point, I have a few requirements:
- none of this crazy stuff
- use a simple inline helper
- make the pointers 'const', because there is no reason not to.
Something *UNTESTED* like the attached diff.
Again: very untested. But at least this generates good code, and
doesn't have pointless crazy variables. Yes, I add that
const struct user_namespace *cred_ns = cred->user_ns;
because while I think gcc may be smart enough to figure out that it's
all the same value, I wanted to make sure.
Then the tracepoint would look something like
trace_cap_capable(cred, targ_ns, cred_ns, cap, opts, ret);
although I don't understand why you'd even trace that 'opts' value
that is never used.
Linus
More information about the Linux-security-module-archive
mailing list