From: Casey Schaufler <casey@schaufler-ca•com>
To: Eric Dumazet <eric.dumazet@gmail•com>
Cc: Paul Moore <pmoore@redhat•com>,
David Miller <davem@davemloft•net>,
netdev@vger•kernel.org, mvadkert@redhat•com,
linux-security-module@vger•kernel.org,
Casey Schaufler <casey@schaufler-ca•com>
Subject: Re: [PATCH] selinux: add a skb_owned_by() hook
Date: Mon, 08 Apr 2013 21:29:35 -0700 [thread overview]
Message-ID: <5163992F.30406@schaufler-ca.com> (raw)
In-Reply-To: <1365479891.3887.99.camel@edumazet-glaptop>
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
>
next prev parent reply other threads:[~2013-04-09 4:29 UTC|newest]
Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-08 15:45 [PATCH] tcp: assign the sock correctly to an outgoing SYNACK packet Paul Moore
2013-04-08 16:14 ` David Miller
2013-04-08 17:22 ` Paul Moore
2013-04-08 17:36 ` Eric Dumazet
2013-04-08 17:40 ` Paul Moore
2013-04-08 17:47 ` Eric Dumazet
2013-04-08 18:01 ` Eric Dumazet
2013-04-08 18:12 ` Paul Moore
2013-04-08 18:21 ` Eric Dumazet
2013-04-08 18:26 ` Paul Moore
2013-04-08 18:34 ` Eric Dumazet
2013-04-08 18:30 ` Eric Dumazet
2013-04-08 20:37 ` Paul Moore
2013-04-08 20:44 ` David Miller
2013-04-08 20:53 ` Paul Moore
2013-04-08 20:55 ` Eric Dumazet
2013-04-08 21:09 ` Paul Moore
2013-04-08 21:14 ` David Miller
2013-04-08 21:17 ` Eric Dumazet
2013-04-09 3:58 ` [PATCH] selinux: add a skb_owned_by() hook Eric Dumazet
2013-04-09 4:29 ` Casey Schaufler [this message]
2013-04-09 4:41 ` David Miller
2013-04-09 5:14 ` Casey Schaufler
2013-04-09 11:39 ` Paul Moore
2013-04-09 6:24 ` Eric Dumazet
2013-04-09 11:45 ` Paul Moore
2013-04-09 7:38 ` James Morris
2013-04-09 12:06 ` Paul Moore
2013-04-09 17:23 ` David Miller
2013-04-08 18:32 ` [PATCH] tcp: assign the sock correctly to an outgoing SYNACK packet Paul Moore
2013-04-08 21:10 ` Paul Moore
2013-04-08 21:15 ` David Miller
2013-04-08 21:24 ` Paul Moore
2013-04-08 21:33 ` David Miller
2013-04-08 22:01 ` Paul Moore
2013-04-08 22:08 ` David Miller
2013-04-08 23:40 ` Casey Schaufler
2013-04-09 0:33 ` Eric Dumazet
2013-04-09 0:59 ` Casey Schaufler
2013-04-09 1:09 ` Eric Dumazet
2013-04-09 1:24 ` Casey Schaufler
2013-04-09 13:19 ` Paul Moore
2013-04-09 13:33 ` Paul Moore
2013-04-09 14:00 ` Eric Dumazet
2013-04-09 14:19 ` Paul Moore
2013-04-09 14:31 ` Eric Dumazet
2013-04-09 14:52 ` Paul Moore
2013-04-09 15:05 ` Paul Moore
2013-04-09 15:07 ` Eric Dumazet
2013-04-09 15:17 ` Paul Moore
2013-04-09 15:32 ` Eric Dumazet
2013-04-09 15:57 ` Paul Moore
2013-04-09 16:11 ` Casey Schaufler
2013-04-09 16:56 ` David Miller
2013-04-09 17:00 ` Paul Moore
2013-04-09 17:09 ` David Miller
2013-04-09 17:10 ` David Miller
2013-04-09 14:05 ` Ben Hutchings
2013-04-09 14:10 ` Paul Moore
2013-04-08 21:34 ` Ben Hutchings
2013-04-08 19:25 ` David Miller
2013-04-08 16:19 ` Eric Dumazet
2013-04-08 18:03 ` Sergei Shtylyov
2013-04-08 18:12 ` Paul Moore
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5163992F.30406@schaufler-ca.com \
--to=casey@schaufler-ca$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=eric.dumazet@gmail$(echo .)com \
--cc=linux-security-module@vger$(echo .)kernel.org \
--cc=mvadkert@redhat$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pmoore@redhat$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox