Re: NULL pointer dereference in selinux_ip_postroute_compat

From: Eric Dumazet
Date: Wed Aug 08 2012 - 16:09:37 EST


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.

> 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.

What kind of security do you envision exactly ?


These unicast_sock are percpu, and preallocated.

/*
* Generic function to send a packet as reply to another packet.
* Used to send some TCP resets/acks so far.
*
* Use a fake percpu inet socket to avoid false sharing and contention.
*/
static DEFINE_PER_CPU(struct inet_sock, unicast_sock) = {
.sk = {
.__sk_common = {
.skc_refcnt = ATOMIC_INIT(1),
},
.sk_wmem_alloc = ATOMIC_INIT(1),
.sk_allocation = GFP_ATOMIC,
.sk_flags = (1UL << SOCK_USE_WRITE_QUEUE),
},
.pmtudisc = IP_PMTUDISC_WANT,
.uc_ttl = -1,
};


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/