[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