Re: [PATCH] selinux: add a skb_owned_by() hook

[Casey Schaufler <casey@schaufler-ca.com>]

On 4/8/2013 8:58 PM, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Commit 90ba9b1986b5ac (tcp: tcp_make_synack() can use alloc_skb())
> broke certain SELinux/NetLabel configurations by no longer correctly
> assigning the sock to the outgoing SYNACK packet.
>
> Cost of atomic operations on the LISTEN socket is quite big,
> and we would like it to happen only if really needed.
>
> This patch introduces a new security_ops->skb_owned_by() method,
> that is a void operation unless selinux is active.

I don't understand what this hook does.
Does it affect Smack (which uses NetLabel) as well?
How can I find out?

>
> Reported-by: Miroslav Vadkerti <mvadkert@redhat.com>
> Diagnosed-by: Paul Moore <pmoore@redhat.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: linux-security-module@vger.kernel.org
> ---
>  include/linux/security.h |    8 ++++++++
>  net/ipv4/tcp_output.c    |    1 +
>  security/capability.c    |    6 ++++++
>  security/security.c      |    5 +++++
>  security/selinux/hooks.c |    7 +++++++
>  5 files changed, 27 insertions(+)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index eee7478..6c3a78a 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1638,6 +1638,7 @@ struct security_operations {
>  	int (*tun_dev_attach_queue) (void *security);
>  	int (*tun_dev_attach) (struct sock *sk, void *security);
>  	int (*tun_dev_open) (void *security);
> +	void (*skb_owned_by) (struct sk_buff *skb, struct sock *sk);
>  #endif	/* CONFIG_SECURITY_NETWORK */
>  
>  #ifdef CONFIG_SECURITY_NETWORK_XFRM
> @@ -2588,6 +2589,8 @@ int security_tun_dev_attach_queue(void *security);
>  int security_tun_dev_attach(struct sock *sk, void *security);
>  int security_tun_dev_open(void *security);
>  
> +void security_skb_owned_by(struct sk_buff *skb, struct sock *sk);
> +
>  #else	/* CONFIG_SECURITY_NETWORK */
>  static inline int security_unix_stream_connect(struct sock *sock,
>  					       struct sock *other,
> @@ -2779,6 +2782,11 @@ static inline int security_tun_dev_open(void *security)
>  {
>  	return 0;
>  }
> +
> +static inline void security_skb_owned_by(struct sk_buff *skb, struct sock *sk)
> +{
> +}
> +
>  #endif	/* CONFIG_SECURITY_NETWORK */
>  
>  #ifdef CONFIG_SECURITY_NETWORK_XFRM
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 5d0b438..b44cf81 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -2709,6 +2709,7 @@ struct sk_buff *tcp_make_synack(struct sock *sk, struct dst_entry *dst,
>  	skb_reserve(skb, MAX_TCP_HEADER);
>  
>  	skb_dst_set(skb, dst);
> +	security_skb_owned_by(skb, sk);
>  
>  	mss = dst_metric_advmss(dst);
>  	if (tp->rx_opt.user_mss && tp->rx_opt.user_mss < mss)
> diff --git a/security/capability.c b/security/capability.c
> index 5797750..c36cca6 100644
> --- a/security/capability.c
> +++ b/security/capability.c
> @@ -737,6 +737,11 @@ static int cap_tun_dev_open(void *security)
>  {
>  	return 0;
>  }
> +
> +static void cap_skb_owned_by(struct sk_buff *skb, struct sock *sk) 
> +{
> +}
> +
>  #endif	/* CONFIG_SECURITY_NETWORK */
>  
>  #ifdef CONFIG_SECURITY_NETWORK_XFRM
> @@ -1071,6 +1076,7 @@ void __init security_fixup_ops(struct security_operations *ops)
>  	set_to_cap_if_null(ops, tun_dev_open);
>  	set_to_cap_if_null(ops, tun_dev_attach_queue);
>  	set_to_cap_if_null(ops, tun_dev_attach);
> +	set_to_cap_if_null(ops, skb_owned_by);
>  #endif	/* CONFIG_SECURITY_NETWORK */
>  #ifdef CONFIG_SECURITY_NETWORK_XFRM
>  	set_to_cap_if_null(ops, xfrm_policy_alloc_security);
> diff --git a/security/security.c b/security/security.c
> index 7b88c6a..03f248b 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1290,6 +1290,11 @@ int security_tun_dev_open(void *security)
>  }
>  EXPORT_SYMBOL(security_tun_dev_open);
>  
> +void security_skb_owned_by(struct sk_buff *skb, struct sock *sk)
> +{
> +	security_ops->skb_owned_by(skb, sk);
> +}
> +
>  #endif	/* CONFIG_SECURITY_NETWORK */
>  
>  #ifdef CONFIG_SECURITY_NETWORK_XFRM
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 2fa28c8..7171a95 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -51,6 +51,7 @@
>  #include <linux/tty.h>
>  #include <net/icmp.h>
>  #include <net/ip.h>		/* for local_port_range[] */
> +#include <net/sock.h>
>  #include <net/tcp.h>		/* struct or_callable used in sock_rcv_skb */
>  #include <net/net_namespace.h>
>  #include <net/netlabel.h>
> @@ -4363,6 +4364,11 @@ static void selinux_inet_conn_established(struct sock *sk, struct sk_buff *skb)
>  	selinux_skb_peerlbl_sid(skb, family, &sksec->peer_sid);
>  }
>  
> +static void selinux_skb_owned_by(struct sk_buff *skb, struct sock *sk)
> +{
> +	skb_set_owner_w(skb, sk);
> +}
> +
>  static int selinux_secmark_relabel_packet(u32 sid)
>  {
>  	const struct task_security_struct *__tsec;
> @@ -5664,6 +5670,7 @@ static struct security_operations selinux_ops = {
>  	.tun_dev_attach_queue =		selinux_tun_dev_attach_queue,
>  	.tun_dev_attach =		selinux_tun_dev_attach,
>  	.tun_dev_open =			selinux_tun_dev_open,
> +	.skb_owned_by =			selinux_skb_owned_by,
>  
>  #ifdef CONFIG_SECURITY_NETWORK_XFRM
>  	.xfrm_policy_alloc_security =	selinux_xfrm_policy_alloc,
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>