On 9/4/19 6:50 PM, Paul Moore wrote:
On Wed, Sep 4, 2019 at 10:32 AM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote:
We need to use selinux_cred() to fetch the SELinux cred blob instead
of directly using current->security or current_security(). There
were a couple of lingering uses of current_security() in the SELinux code
that were apparently missed during the earlier conversions. IIUC, this
would only manifest as a bug if multiple security modules including
SELinux are enabled and SELinux is not first in the lsm order. After
this change, there appear to be no other users of current_security()
in-tree; perhaps we should remove it altogether.
Fixes: bbd3662a8348 ("Infrastructure management of the cred security blob")
Signed-off-by: Stephen Smalley <sds@xxxxxxxxxxxxx>
---
security/selinux/hooks.c | 2 +-
security/selinux/include/objsec.h | 20 ++++++++++----------
2 files changed, 11 insertions(+), 11 deletions(-)
Thanks Stephen, and everyone who reviewed/commented on the patch.
This looks fine to me too, and while it is a little late, I think
there is value in getting this into the next merge window so I've gone
ahead and merged this into selinux/next.
As far as removing current_security is concerned, I also agree that
removing it is probably a good idea. Does anyone object if I merge a
follow-up patch via the SELinux tree (patch coming shortly)?
Obviously no objection to the follow-up patch.
I had another concern though raised by this bug that I wanted to note.
AFAICS, no SELinux maintainer ever acked or reviewed the commit that
introduced the bug, or the other patches in that series. In fact that
commit and many others in the series only have a single Reviewed-by line
and no Acked-by lines at all. I guess the SELinux maintainers (self
included) could/should have been more proactive but maybe there should
have been a final poke at each of the security module maintainers to
provide at least an ack before it was merged or at least sent up to Linus?
I've also seen a tendency to assume that passing the selinux-testsuite
suffices for ensuring that the patch is correct wrt SELinux. If it
wasn't already clear, the selinux-testsuite is by no means complete in
its coverage (contributions welcome; known gaps captured as open issues
in github), so passing the testsuite is no substitute for code review.
For the next and any future rounds of stacking support, I'm hoping we
can be a bit more rigorous in our code review and testing requirements.
