[GIT PULL] SELinux list corruption fix for 3.18

[GIT PULL] SELinux list corruption fix for 3.18

[James Morris]

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 <sds@tycho.nsa.gov>
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 <shivnandan.k@samsung.com>
    Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
    Cc: stable@vger.kernel.org
    Signed-off-by: Paul Moore <pmoore@redhat.com>

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);

Re: [GIT PULL] SELinux list corruption fix for 3.18

[Linus Torvalds]

On Tue, Oct 14, 2014 at 5:05 PM, James Morris <jmorris@namei.org> 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

Re: [GIT PULL] SELinux list corruption fix for 3.18

[Linus Torvalds]

On Wed, Oct 15, 2014 at 7:27 AM, Linus Torvalds
<torvalds@linux-foundation.org> 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

Re: [GIT PULL] SELinux list corruption fix for 3.18

[James Morris]

On Wed, 15 Oct 2014, Linus Torvalds wrote:

> On Wed, Oct 15, 2014 at 7:27 AM, Linus Torvalds
> <torvalds@linux-foundation.org> 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 <sds@tycho.nsa.gov>
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
<jmorris@namei.org>

Re: [GIT PULL] SELinux list corruption fix for 3.18

[Paul Moore]

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
> > 
> > <torvalds@linux-foundation.org> 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 <sds@tycho.nsa.gov>
> 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
www.paul-moore.com