Comment 16 for bug 1897369

Revision history for this message
Joel Holveck (joelh) wrote :

> Should that be fixed in the Linux kernel, or at least reported to the developers?

tl;dr: I don't have insight to the relevant community, but it may be worth reporting to either the kernel community or the apparmor devs.

Context: I'm just a random user who did the analysis, not an Ubuntu or Debian Developer.

I mentioned this in the Debian bug (debbugs#1016622). I can see either way that this would go; it could be a disconnect between the devs who did the capability framework vs. the devs who implemented apparmor.

Is capable() meant to be something that can have side effects? The people who wrote __sched_setscheduler clearly expected that it wouldn't, while the apparmor devs did put a side effect there (the only place they could put audit logs).

If you restructure that code to make it a bunch of `if (condition && capable(CAP_SYS_NICE))` tests, then you run the risk of not testing `capable`, or getting the order wrong. If you don't restructure that code, then you have this issue that there's no place to audit for when a capability is actually needed.

You could also extend the kernel capability API to have separate test and audit functions, so you could build that section as `if (!capable_test_only(CAP_SYS_NICE)) { ... if (condition) return capability_audit_and_return(CAP_SYS_NICE, -EPERM); ... }`, but I don't see that as particularly useful.

As I said, this seems to be a disconnect in whether or not capable() should have side effects; maybe the apparmor guys would be best positioned to engage the kernel community on the matter.

That's my thoughts, but I tried not to give recommendations. I'm not active in the Linux kernel development community, and I don't know what their preferred style is. I couldn't find any docs on the capability API (I didn't look terribly hard), so I don't know if there's documentation to discuss whether or not capable() is supposed to have side-effects.