[PATCH] SELinux: BUG in SELinux compat_net code

[PATCH] SELinux: BUG in SELinux compat_net code

[Eric Paris]

This patch is not applicable to Linus's tree as the code in question has
been removed for 2.6.30.  I'm sending in case any of the stable
maintainers would like to push to their branches (which I think anything
pre 2.6.30 would like to do).

Ubuntu users were experiencing a kernel panic when they enabled SELinux
due to an old bug in our handling of the compatibility mode network
controls, introduced Jan 1 2008 effad8df44261031a882e1a895415f7186a5098e
Most distros have not used the compat_net code since the new code was
introduced and so noone has hit this problem before.  Ubuntu is the only
distro I know that enabled that legacy cruft by default.  But, I was ask
to look at it and found that the above patch changed a call to
avc_has_perm from if(send_perm) to if(!send_perm) in
selinux_ip_postroute_iptables_compat().  The result is that users who
turn on SELinux and have compat_net set can (and oftern will) BUG() in
avc_has_perm_noaudit since they are requesting 0 permissions.

This patch corrects that accidental bug introduction.

Signed-off-by: Eric Paris <eparis@redhat.com>

---

 security/selinux/hooks.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -up linux-source-2.6.28/security/selinux/hooks.c.pre.send linux-source-2.6.28/security/selinux/hooks.c
--- linux-source-2.6.28/security/selinux/hooks.c.pre.send	2009-05-18 13:23:16.043632602 -0400
+++ linux-source-2.6.28/security/selinux/hooks.c	2009-05-18 13:23:27.899632772 -0400
@@ -4561,7 +4561,7 @@ static int selinux_ip_postroute_iptables
 	if (err)
 		return err;
 
-	if (send_perm != 0)
+	if (!send_perm)
 		return 0;
 
 	err = sel_netport_sid(sk->sk_protocol,

Re: [PATCH] SELinux: BUG in SELinux compat_net code

[Eric Paris]

Sometimes I'm an idiot, messed up TWO e-mail addresses....   stable
and selinux, so I'm hitting both of those lists with this reply...

On Tue, May 19, 2009 at 5:41 PM, Eric Paris <eparis@redhat.com> wrote:
> This patch is not applicable to Linus's tree as the code in question has
> been removed for 2.6.30.  I'm sending in case any of the stable
> maintainers would like to push to their branches (which I think anything
> pre 2.6.30 would like to do).
>
> Ubuntu users were experiencing a kernel panic when they enabled SELinux
> due to an old bug in our handling of the compatibility mode network
> controls, introduced Jan 1 2008 effad8df44261031a882e1a895415f7186a5098e
> Most distros have not used the compat_net code since the new code was
> introduced and so noone has hit this problem before.  Ubuntu is the only
> distro I know that enabled that legacy cruft by default.  But, I was ask
> to look at it and found that the above patch changed a call to
> avc_has_perm from if(send_perm) to if(!send_perm) in
> selinux_ip_postroute_iptables_compat().  The result is that users who
> turn on SELinux and have compat_net set can (and oftern will) BUG() in
> avc_has_perm_noaudit since they are requesting 0 permissions.
>
> This patch corrects that accidental bug introduction.
>
> Signed-off-by: Eric Paris <eparis@redhat.com>
>
> ---
>
>  security/selinux/hooks.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff -up linux-source-2.6.28/security/selinux/hooks.c.pre.send linux-source-2.6.28/security/selinux/hooks.c
> --- linux-source-2.6.28/security/selinux/hooks.c.pre.send       2009-05-18 13:23:16.043632602 -0400
> +++ linux-source-2.6.28/security/selinux/hooks.c        2009-05-18 13:23:27.899632772 -0400
> @@ -4561,7 +4561,7 @@ static int selinux_ip_postroute_iptables
>        if (err)
>                return err;
>
> -       if (send_perm != 0)
> +       if (!send_perm)
>                return 0;
>
>        err = sel_netport_sid(sk->sk_protocol,
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: [PATCH] SELinux: BUG in SELinux compat_net code

[Paul Moore]

On Tuesday 19 May 2009 05:41:58 pm Eric Paris wrote:
> This patch is not applicable to Linus's tree as the code in question has
> been removed for 2.6.30.  I'm sending in case any of the stable
> maintainers would like to push to their branches (which I think anything
> pre 2.6.30 would like to do).
>
> Ubuntu users were experiencing a kernel panic when they enabled SELinux
> due to an old bug in our handling of the compatibility mode network
> controls, introduced Jan 1 2008 effad8df44261031a882e1a895415f7186a5098e
> Most distros have not used the compat_net code since the new code was
> introduced and so noone has hit this problem before.  Ubuntu is the only
> distro I know that enabled that legacy cruft by default.  But, I was ask
> to look at it and found that the above patch changed a call to
> avc_has_perm from if(send_perm) to if(!send_perm) in
> selinux_ip_postroute_iptables_compat().  The result is that users who
> turn on SELinux and have compat_net set can (and oftern will) BUG() in
> avc_has_perm_noaudit since they are requesting 0 permissions.
>
> This patch corrects that accidental bug introduction.
>
> Signed-off-by: Eric Paris <eparis@redhat.com>

My mistake, thanks to Eric for catching and fixing this bug.

Acked-by: Paul Moore <paul.moore@hp.com>

> ---
>
>  security/selinux/hooks.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff -up linux-source-2.6.28/security/selinux/hooks.c.pre.send
> linux-source-2.6.28/security/selinux/hooks.c ---
> linux-source-2.6.28/security/selinux/hooks.c.pre.send	2009-05-18
> 13:23:16.043632602 -0400 +++
> linux-source-2.6.28/security/selinux/hooks.c	2009-05-18 13:23:27.899632772
> -0400 @@ -4561,7 +4561,7 @@ static int selinux_ip_postroute_iptables
>  	if (err)
>  		return err;
>
> -	if (send_perm != 0)
> +	if (!send_perm)
>  		return 0;
>
>  	err = sel_netport_sid(sk->sk_protocol,

-- 
paul moore
linux @ hp

Re: [PATCH] SELinux: BUG in SELinux compat_net code

[Paul Moore]

On Tuesday 19 May 2009 05:53:52 pm Eric Paris wrote:
> Sometimes I'm an idiot, messed up TWO e-mail addresses....   stable
> and selinux, so I'm hitting both of those lists with this reply...

... and I fell for it too when ack'ing the patch.  I may write buggy code but 
at least I submit it to the write addresses ;)

Anyway, looks good to me.

Acked-by: Paul Moore <paul.moore@hp.com>

> On Tue, May 19, 2009 at 5:41 PM, Eric Paris <eparis@redhat.com> wrote:
> > This patch is not applicable to Linus's tree as the code in question has
> > been removed for 2.6.30.  I'm sending in case any of the stable
> > maintainers would like to push to their branches (which I think anything
> > pre 2.6.30 would like to do).
> >
> > Ubuntu users were experiencing a kernel panic when they enabled SELinux
> > due to an old bug in our handling of the compatibility mode network
> > controls, introduced Jan 1 2008 effad8df44261031a882e1a895415f7186a5098e
> > Most distros have not used the compat_net code since the new code was
> > introduced and so noone has hit this problem before.  Ubuntu is the only
> > distro I know that enabled that legacy cruft by default.  But, I was ask
> > to look at it and found that the above patch changed a call to
> > avc_has_perm from if(send_perm) to if(!send_perm) in
> > selinux_ip_postroute_iptables_compat().  The result is that users who
> > turn on SELinux and have compat_net set can (and oftern will) BUG() in
> > avc_has_perm_noaudit since they are requesting 0 permissions.
> >
> > This patch corrects that accidental bug introduction.
> >
> > Signed-off-by: Eric Paris <eparis@redhat.com>
> >
> > ---
> >
> >  security/selinux/hooks.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff -up linux-source-2.6.28/security/selinux/hooks.c.pre.send
> > linux-source-2.6.28/security/selinux/hooks.c ---
> > linux-source-2.6.28/security/selinux/hooks.c.pre.send       2009-05-18
> > 13:23:16.043632602 -0400 +++ linux-source-2.6.28/security/selinux/hooks.c
> >        2009-05-18 13:23:27.899632772 -0400 @@ -4561,7 +4561,7 @@ static
> > int selinux_ip_postroute_iptables
> >        if (err)
> >                return err;
> >
> > -       if (send_perm != 0)
> > +       if (!send_perm)
> >                return 0;
> >
> >        err = sel_netport_sid(sk->sk_protocol,
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel"
> > in the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/

-- 
paul moore
linux @ hp