> 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.
> 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.