lkml.org 
[lkml]   [2012]   [Aug]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: NULL pointer dereference in selinux_ip_postroute_compat
Date
On Wednesday, August 08, 2012 10:09:38 PM Eric Dumazet wrote:
> On Wed, 2012-08-08 at 15:59 -0400, Eric Paris wrote:
> > Seems wrong. We shouldn't ever need ifdef CONFIG_SECURITY in core
> > code.
>
> Sure but it seems include file misses an accessor for this.
>
> We could add it on a future cleanup patch, as Paul mentioned.

Actually, the issue is that the shared socket doesn't have an init/alloc
function to do the LSM allocation like we do with other sockets so Eric's
patch does it as part of ip_send_unicast_reply().

If we look at the relevant part of Eric's patch:

+#ifdef CONFIG_SECURITY
+ if (!sk->sk_security && security_sk_alloc(sk, PF_INET, GFP_ATOMIC))
+ goto out;
+#endif
... if we were to remove the CONFIG_SECURITY conditional we would end up
calling security_sk_alloc() each time through in the CONFIG_SECURITY=n case as
sk->sk_security would never be initialized to a non-NULL value. In the
CONFIG_SECURITY=y case it should only be called once as security_sk_alloc()
should set sk->sk_security to a LSM blob.

> > Ifndef CONF_SECURITY then security_sk_alloc() is a static
> >
> > inline return 0; I guess the question is "Where did the sk come
> > from"? Why wasn't security_sk_alloc() called when it was allocated?
> > Should it have been updated at some time and that wasn't done either?
> > Seems wrong to be putting packets on the queue for a socket where the
> > security data was never allocated and was never set to its proper
> > state.
>
> IMHO it seems wrong to even care about security for internal sockets.
>
> They are per cpu, shared for all users on the machine.

The issue, from a security point of view, is that these sockets are sending
network traffic; even if it is just resets and timewait ACKs, it is still
network traffic and the LSMs need to be able to enforce security policy on
this traffic. After all, what would you say if your firewall let these same
packets pass without any filtering?

The issue I'm struggling with at present is how should we handle this traffic
from a LSM perspective. The label based LSMs, e.g. SELinux and Smack, use the
LSM blob assigned to locally generated outbound traffic to identify the
traffic and apply the security policy, so not only do we have to resolve the
issue of ensuring the traffic is labeled correctly, we have to do it with a
shared socket (although the patch didn't change the shared nature of the
socket).

For those who are interested, I think the reasonable labeling solution here is
to go with SECINITSID_KERNEL/kernel_t for SELinux and likely the ambient label
for Smack as in both the TCP reset and timewait ACK there shouldn't be any
actual user data present.

--
paul moore
www.paul-moore.com



\
 
 \ /
  Last update: 2012-08-08 23:21    [from the cache]
©2003-2011 Jasper Spaans