This is a fix for list corruption in the SELinux code.
Note that the git-log output is still broken due to the back-merge issue
previously discussed. The fix is in commit
7c66bdc72bc3d792886c42bbab4b214c1fe536e0
Please pull.
--
The following changes since commit 2d65a9f48fcdf7866aab6457bc707ca233e0c791:
Merge branch 'drm-next' of git://people.freedesktop.org/~airlied/linux (2014-10-14 09:39:08 +0200)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git for-linus
James Morris (1):
Merge branch 'stable-3.18' of git://git.infradead.org/users/pcmoore/selinux into for-linus
Jiri Pirko (1):
selinux: register nf hooks with single nf_register_hooks call
Paul Moore (2):
selinux: fix a problem with IPv6 traffic denials in selinux_ip_postroute()
selinux: make the netif cache namespace aware
Richard Guy Briggs (2):
selinux: cleanup error reporting in selinux_nlmsg_perm()
selinux: normalize audit log formatting
Stephen Smalley (2):
selinux: Permit bounded transitions under NO_NEW_PRIVS or NOSUID.
selinux: fix inode security list corruption
security/selinux/hooks.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
commit 7c66bdc72bc3d792886c42bbab4b214c1fe536e0
Author: Stephen Smalley <[email protected]>
Date: Mon Oct 6 16:32:52 2014 -0400
selinux: fix inode security list corruption
sb_finish_set_opts() can race with inode_free_security()
when initializing inode security structures for inodes
created prior to initial policy load or by the filesystem
during ->mount(). This appears to have always been
a possible race, but commit 3dc91d4 ("SELinux: Fix possible
NULL pointer dereference in selinux_inode_permission()")
made it more evident by immediately reusing the unioned
list/rcu element of the inode security structure for call_rcu()
upon an inode_free_security(). But the underlying issue
was already present before that commit as a possible use-after-free
of isec.
Shivnandan Kumar reported the list corruption and proposed
a patch to split the list and rcu elements out of the union
as separate fields of the inode_security_struct so that setting
the rcu element would not affect the list element. However,
this would merely hide the issue and not truly fix the code.
This patch instead moves up the deletion of the list entry
prior to dropping the sbsec->isec_lock initially. Then,
if the inode is dropped subsequently, there will be no further
references to the isec.
Reported-by: Shivnandan Kumar <[email protected]>
Signed-off-by: Stephen Smalley <[email protected]>
Cc: [email protected]
Signed-off-by: Paul Moore <[email protected]>
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 29e64d4..2478976 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -481,6 +481,7 @@ next_inode:
list_entry(sbsec->isec_head.next,
struct inode_security_struct, list);
struct inode *inode = isec->inode;
+ list_del_init(&isec->list);
spin_unlock(&sbsec->isec_lock);
inode = igrab(inode);
if (inode) {
@@ -489,7 +490,6 @@ next_inode:
iput(inode);
}
spin_lock(&sbsec->isec_lock);
- list_del_init(&isec->list);
goto next_inode;
}
spin_unlock(&sbsec->isec_lock);
On Tue, Oct 14, 2014 at 5:05 PM, James Morris <[email protected]> wrote:
> This is a fix for list corruption in the SELinux code.
>
> Note that the git-log output is still broken due to the back-merge issue
> previously discussed. The fix is in commit
> 7c66bdc72bc3d792886c42bbab4b214c1fe536e0
No, the log is correct (the log always is, the back-merges can cause
the trivial *diff* to be broken).
Your branch has way more than that one list corruption fix in it.
Please fix your branch, or fix your description.
Linus
On Wed, Oct 15, 2014 at 7:27 AM, Linus Torvalds
<[email protected]> wrote:
>
> No, the log is correct (the log always is, the back-merges can cause
> the trivial *diff* to be broken).
To add some details: the commits in there are
f6ed66087648 Merge branch 'stable-3.18' of
git://git.infradead.org/users/pcmoore/selinux into f
7c66bdc72bc3 selinux: fix inode security list corruption
e7387395a07d selinux: normalize audit log formatting
8497b78ecc9d selinux: cleanup error reporting in selinux_nlmsg_perm()
6eb1ddc6bd3f selinux: make the netif cache namespace aware
5e29532fed21 selinux: register nf hooks with single nf_register_hooks call
82341ad9b962 selinux: fix a problem with IPv6 traffic denials in
selinux_ip_postroute()
04e8d6ab1fa6 selinux: Permit bounded transitions under NO_NEW_PRIVS or NOSUID.
and all but the list corruption fix seem to just be duplicate commits
of things I have already gotten elsewhere. Somebody cherry-picking
commits and duplicating them?
So there are more things rotten in this tree than a back-merge.
Linus
On Wed, 15 Oct 2014, Linus Torvalds wrote:
> On Wed, Oct 15, 2014 at 7:27 AM, Linus Torvalds
> <[email protected]> wrote:
> >
> > No, the log is correct (the log always is, the back-merges can cause
> > the trivial *diff* to be broken).
>
> To add some details: the commits in there are
>
> f6ed66087648 Merge branch 'stable-3.18' of
> git://git.infradead.org/users/pcmoore/selinux into f
> 7c66bdc72bc3 selinux: fix inode security list corruption
> e7387395a07d selinux: normalize audit log formatting
> 8497b78ecc9d selinux: cleanup error reporting in selinux_nlmsg_perm()
> 6eb1ddc6bd3f selinux: make the netif cache namespace aware
> 5e29532fed21 selinux: register nf hooks with single nf_register_hooks call
> 82341ad9b962 selinux: fix a problem with IPv6 traffic denials in
> selinux_ip_postroute()
> 04e8d6ab1fa6 selinux: Permit bounded transitions under NO_NEW_PRIVS or NOSUID.
>
> and all but the list corruption fix seem to just be duplicate commits
> of things I have already gotten elsewhere. Somebody cherry-picking
> commits and duplicating them?
>
Yep, I also already have these other changes under different commit IDs:
commit 7b0d0b40cd78cadb525df760ee4cac151533c2b5
Author: Stephen Smalley <[email protected]>
Date: Mon Aug 4 13:36:49 2014 -0400
selinux: Permit bounded transitions under NO_NEW_PRIVS or NOSUID.
Paul: do you have the above commit ID in your tree?
> So there are more things rotten in this tree than a back-merge.
--
James Morris
<[email protected]>
On Wednesday, October 15, 2014 07:05:55 PM James Morris wrote:
> On Wed, 15 Oct 2014, Linus Torvalds wrote:
> > On Wed, Oct 15, 2014 at 7:27 AM, Linus Torvalds
> >
> > <[email protected]> wrote:
> > > No, the log is correct (the log always is, the back-merges can cause
> > > the trivial *diff* to be broken).
> >
> > To add some details: the commits in there are
> >
> > f6ed66087648 Merge branch 'stable-3.18' of
> >
> > git://git.infradead.org/users/pcmoore/selinux into f
> >
> > 7c66bdc72bc3 selinux: fix inode security list corruption
> > e7387395a07d selinux: normalize audit log formatting
> > 8497b78ecc9d selinux: cleanup error reporting in selinux_nlmsg_perm()
> > 6eb1ddc6bd3f selinux: make the netif cache namespace aware
> > 5e29532fed21 selinux: register nf hooks with single nf_register_hooks
> > call
> > 82341ad9b962 selinux: fix a problem with IPv6 traffic denials in
> >
> > selinux_ip_postroute()
> >
> > 04e8d6ab1fa6 selinux: Permit bounded transitions under NO_NEW_PRIVS or
> > NOSUID.>
> > and all but the list corruption fix seem to just be duplicate commits
> > of things I have already gotten elsewhere. Somebody cherry-picking
> > commits and duplicating them?
>
> Yep, I also already have these other changes under different commit IDs:
>
> commit 7b0d0b40cd78cadb525df760ee4cac151533c2b5
> Author: Stephen Smalley <[email protected]>
> Date: Mon Aug 4 13:36:49 2014 -0400
>
> selinux: Permit bounded transitions under NO_NEW_PRIVS or NOSUID.
>
>
> Paul: do you have the above commit ID in your tree?
It would seem I've got the Midas touch this merge window, my apologies yet
again.
The short version is that everything should be fixed now, the stable-3.18
branch should be the single list corruption patch on top of the next branch I
sent previously. James, could you please re-pull from the stable-3.18 branch?
Thanks for your patience.
The slightly longer version is that when I created the stable-3.18 branch from
the next branch I used the '--clone' flag in stgit which updated the dates on
the patches which were new for #next (not "committed" in stgit) and caused the
commit IDs to change. Another lesson learned, I'm sorry it caused additional
problems.
-Paul
--
paul moore
http://www.paul-moore.com