From 06198b34a3e09e06d9aecaa3727e0d37206cea77 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Fri, 18 Sep 2015 14:33:06 -0500 Subject: netfilter: Pass priv instead of nf_hook_ops to netfilter hooks Only pass the void *priv parameter out of the nf_hook_ops. That is all any of the functions are interested now, and by limiting what is passed it becomes simpler to change implementation details. Signed-off-by: "Eric W. Biederman" Signed-off-by: Pablo Neira Ayuso --- security/selinux/hooks.c | 10 +++++----- security/smack/smack_netfilter.c | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index e4369d86e588..64340160f4ac 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -4866,7 +4866,7 @@ static unsigned int selinux_ip_forward(struct sk_buff *skb, return NF_ACCEPT; } -static unsigned int selinux_ipv4_forward(const struct nf_hook_ops *ops, +static unsigned int selinux_ipv4_forward(void *priv, struct sk_buff *skb, const struct nf_hook_state *state) { @@ -4874,7 +4874,7 @@ static unsigned int selinux_ipv4_forward(const struct nf_hook_ops *ops, } #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE) -static unsigned int selinux_ipv6_forward(const struct nf_hook_ops *ops, +static unsigned int selinux_ipv6_forward(void *priv, struct sk_buff *skb, const struct nf_hook_state *state) { @@ -4924,7 +4924,7 @@ static unsigned int selinux_ip_output(struct sk_buff *skb, return NF_ACCEPT; } -static unsigned int selinux_ipv4_output(const struct nf_hook_ops *ops, +static unsigned int selinux_ipv4_output(void *priv, struct sk_buff *skb, const struct nf_hook_state *state) { @@ -5099,7 +5099,7 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, return NF_ACCEPT; } -static unsigned int selinux_ipv4_postroute(const struct nf_hook_ops *ops, +static unsigned int selinux_ipv4_postroute(void *priv, struct sk_buff *skb, const struct nf_hook_state *state) { @@ -5107,7 +5107,7 @@ static unsigned int selinux_ipv4_postroute(const struct nf_hook_ops *ops, } #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE) -static unsigned int selinux_ipv6_postroute(const struct nf_hook_ops *ops, +static unsigned int selinux_ipv6_postroute(void *priv, struct sk_buff *skb, const struct nf_hook_state *state) { diff --git a/security/smack/smack_netfilter.c b/security/smack/smack_netfilter.c index a455cfc9ec1f..a9e41da05d28 100644 --- a/security/smack/smack_netfilter.c +++ b/security/smack/smack_netfilter.c @@ -21,7 +21,7 @@ #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE) -static unsigned int smack_ipv6_output(const struct nf_hook_ops *ops, +static unsigned int smack_ipv6_output(void *priv, struct sk_buff *skb, const struct nf_hook_state *state) { @@ -38,7 +38,7 @@ static unsigned int smack_ipv6_output(const struct nf_hook_ops *ops, } #endif /* IPV6 */ -static unsigned int smack_ipv4_output(const struct nf_hook_ops *ops, +static unsigned int smack_ipv4_output(void *priv, struct sk_buff *skb, const struct nf_hook_state *state) { -- cgit v1.2.3 From e446f9dfe17bbaa76a1fe22912636f38be1e1af8 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Thu, 8 Oct 2015 05:01:55 -0700 Subject: net: synack packets can be attached to request sockets selinux needs few changes to accommodate fact that SYNACK messages can be attached to a request socket, lacking sk_security pointer (Only syncookies are still attached to a TCP_LISTEN socket) Adds a new sk_listener() helper, and use it in selinux and sch_fq Fixes: ca6fb0651883 ("tcp: attach SYNACK messages to request sockets instead of listener") Signed-off-by: Eric Dumazet Reported by: kernel test robot Cc: Paul Moore Cc: Stephen Smalley Cc: Eric Paris Acked-by: Paul Moore Signed-off-by: David S. Miller --- security/selinux/hooks.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 64340160f4ac..6e50841ef1f6 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -4898,7 +4898,7 @@ static unsigned int selinux_ip_output(struct sk_buff *skb, if (sk) { struct sk_security_struct *sksec; - if (sk->sk_state == TCP_LISTEN) + if (sk_listener(sk)) /* if the socket is the listening state then this * packet is a SYN-ACK packet which means it needs to * be labeled based on the connection/request_sock and @@ -5005,7 +5005,7 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, * unfortunately, this means more work, but it is only once per * connection. */ if (skb_dst(skb) != NULL && skb_dst(skb)->xfrm != NULL && - !(sk != NULL && sk->sk_state == TCP_LISTEN)) + !(sk && sk_listener(sk))) return NF_ACCEPT; #endif @@ -5022,7 +5022,7 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, secmark_perm = PACKET__SEND; peer_sid = SECINITSID_KERNEL; } - } else if (sk->sk_state == TCP_LISTEN) { + } else if (sk_listener(sk)) { /* Locally generated packet but the associated socket is in the * listening state which means this is a SYN-ACK packet. In * this particular case the correct security label is assigned @@ -5033,7 +5033,11 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, * selinux_inet_conn_request(). See also selinux_ip_output() * for similar problems. */ u32 skb_sid; - struct sk_security_struct *sksec = sk->sk_security; + struct sk_security_struct *sksec; + + if (sk->sk_state == TCP_NEW_SYN_RECV) + sk = inet_reqsk(sk)->rsk_listener; + sksec = sk->sk_security; if (selinux_skb_peerlbl_sid(skb, family, &skb_sid)) return NF_DROP; /* At this point, if the returned skb peerlbl is SECSID_NULL -- cgit v1.2.3 From f05819df10d7b09f6d1eb6f8534a8f68e5a4fe61 Mon Sep 17 00:00:00 2001 From: David Howells Date: Thu, 15 Oct 2015 17:21:37 +0100 Subject: KEYS: Fix crash when attempt to garbage collect an uninstantiated keyring The following sequence of commands: i=`keyctl add user a a @s` keyctl request2 keyring foo bar @t keyctl unlink $i @s tries to invoke an upcall to instantiate a keyring if one doesn't already exist by that name within the user's keyring set. However, if the upcall fails, the code sets keyring->type_data.reject_error to -ENOKEY or some other error code. When the key is garbage collected, the key destroy function is called unconditionally and keyring_destroy() uses list_empty() on keyring->type_data.link - which is in a union with reject_error. Subsequently, the kernel tries to unlink the keyring from the keyring names list - which oopses like this: BUG: unable to handle kernel paging request at 00000000ffffff8a IP: [] keyring_destroy+0x3d/0x88 ... Workqueue: events key_garbage_collector ... RIP: 0010:[] keyring_destroy+0x3d/0x88 RSP: 0018:ffff88003e2f3d30 EFLAGS: 00010203 RAX: 00000000ffffff82 RBX: ffff88003bf1a900 RCX: 0000000000000000 RDX: 0000000000000000 RSI: 000000003bfc6901 RDI: ffffffff81a73a40 RBP: ffff88003e2f3d38 R08: 0000000000000152 R09: 0000000000000000 R10: ffff88003e2f3c18 R11: 000000000000865b R12: ffff88003bf1a900 R13: 0000000000000000 R14: ffff88003bf1a908 R15: ffff88003e2f4000 ... CR2: 00000000ffffff8a CR3: 000000003e3ec000 CR4: 00000000000006f0 ... Call Trace: [] key_gc_unused_keys.constprop.1+0x5d/0x10f [] key_garbage_collector+0x1fa/0x351 [] process_one_work+0x28e/0x547 [] worker_thread+0x26e/0x361 [] ? rescuer_thread+0x2a8/0x2a8 [] kthread+0xf3/0xfb [] ? kthread_create_on_node+0x1c2/0x1c2 [] ret_from_fork+0x3f/0x70 [] ? kthread_create_on_node+0x1c2/0x1c2 Note the value in RAX. This is a 32-bit representation of -ENOKEY. The solution is to only call ->destroy() if the key was successfully instantiated. Reported-by: Dmitry Vyukov Signed-off-by: David Howells Tested-by: Dmitry Vyukov --- security/keys/gc.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/keys/gc.c b/security/keys/gc.c index 39eac1fd5706..addf060399e0 100644 --- a/security/keys/gc.c +++ b/security/keys/gc.c @@ -134,8 +134,10 @@ static noinline void key_gc_unused_keys(struct list_head *keys) kdebug("- %u", key->serial); key_check(key); - /* Throw away the key data */ - if (key->type->destroy) + /* Throw away the key data if the key is instantiated */ + if (test_bit(KEY_FLAG_INSTANTIATED, &key->flags) && + !test_bit(KEY_FLAG_NEGATIVE, &key->flags) && + key->type->destroy) key->type->destroy(key); security_key_free(key); -- cgit v1.2.3 From 2ffbceb2b08f8ca0496c54a9ebcd11d25275954e Mon Sep 17 00:00:00 2001 From: Florian Westphal Date: Tue, 13 Oct 2015 14:33:26 +0200 Subject: netfilter: remove hook owner refcounting since commit 8405a8fff3f8 ("netfilter: nf_qeueue: Drop queue entries on nf_unregister_hook") all pending queued entries are discarded. So we can simply remove all of the owner handling -- when module is removed it also needs to unregister all its hooks. Signed-off-by: Florian Westphal Signed-off-by: Pablo Neira Ayuso --- security/selinux/hooks.c | 5 ----- security/smack/smack_netfilter.c | 2 -- 2 files changed, 7 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 64340160f4ac..659bb50f0232 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6127,21 +6127,18 @@ security_initcall(selinux_init); static struct nf_hook_ops selinux_nf_ops[] = { { .hook = selinux_ipv4_postroute, - .owner = THIS_MODULE, .pf = NFPROTO_IPV4, .hooknum = NF_INET_POST_ROUTING, .priority = NF_IP_PRI_SELINUX_LAST, }, { .hook = selinux_ipv4_forward, - .owner = THIS_MODULE, .pf = NFPROTO_IPV4, .hooknum = NF_INET_FORWARD, .priority = NF_IP_PRI_SELINUX_FIRST, }, { .hook = selinux_ipv4_output, - .owner = THIS_MODULE, .pf = NFPROTO_IPV4, .hooknum = NF_INET_LOCAL_OUT, .priority = NF_IP_PRI_SELINUX_FIRST, @@ -6149,14 +6146,12 @@ static struct nf_hook_ops selinux_nf_ops[] = { #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE) { .hook = selinux_ipv6_postroute, - .owner = THIS_MODULE, .pf = NFPROTO_IPV6, .hooknum = NF_INET_POST_ROUTING, .priority = NF_IP6_PRI_SELINUX_LAST, }, { .hook = selinux_ipv6_forward, - .owner = THIS_MODULE, .pf = NFPROTO_IPV6, .hooknum = NF_INET_FORWARD, .priority = NF_IP6_PRI_SELINUX_FIRST, diff --git a/security/smack/smack_netfilter.c b/security/smack/smack_netfilter.c index a9e41da05d28..6d1706c9777e 100644 --- a/security/smack/smack_netfilter.c +++ b/security/smack/smack_netfilter.c @@ -57,7 +57,6 @@ static unsigned int smack_ipv4_output(void *priv, static struct nf_hook_ops smack_nf_ops[] = { { .hook = smack_ipv4_output, - .owner = THIS_MODULE, .pf = NFPROTO_IPV4, .hooknum = NF_INET_LOCAL_OUT, .priority = NF_IP_PRI_SELINUX_FIRST, @@ -65,7 +64,6 @@ static struct nf_hook_ops smack_nf_ops[] = { #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE) { .hook = smack_ipv6_output, - .owner = THIS_MODULE, .pf = NFPROTO_IPV6, .hooknum = NF_INET_LOCAL_OUT, .priority = NF_IP6_PRI_SELINUX_FIRST, -- cgit v1.2.3 From 911b79cde95c7da0ec02f48105358a36636b7a71 Mon Sep 17 00:00:00 2001 From: David Howells Date: Mon, 19 Oct 2015 11:20:28 +0100 Subject: KEYS: Don't permit request_key() to construct a new keyring If request_key() is used to find a keyring, only do the search part - don't do the construction part if the keyring was not found by the search. We don't really want keyrings in the negative instantiated state since the rejected/negative instantiation error value in the payload is unioned with keyring metadata. Now the kernel gives an error: request_key("keyring", "#selinux,bdekeyring", "keyring", KEY_SPEC_USER_SESSION_KEYRING) = -1 EPERM (Operation not permitted) Signed-off-by: David Howells --- security/keys/request_key.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'security') diff --git a/security/keys/request_key.c b/security/keys/request_key.c index 486ef6fa393b..0d6253124278 100644 --- a/security/keys/request_key.c +++ b/security/keys/request_key.c @@ -440,6 +440,9 @@ static struct key *construct_key_and_link(struct keyring_search_context *ctx, kenter(""); + if (ctx->index_key.type == &key_type_keyring) + return ERR_PTR(-EPERM); + user = key_user_lookup(current_fsuid()); if (!user) return ERR_PTR(-ENOMEM); -- cgit v1.2.3 From 212cd0895330b775f2db49451f046a5ca4e5704b Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Thu, 5 Nov 2015 13:39:24 -0800 Subject: selinux: fix random read in selinux_ip_postroute_compat() In commit e446f9dfe17b ("net: synack packets can be attached to request sockets"), I missed one remaining case of invalid skb->sk->sk_security access. Dmitry Vyukov got a KASan report pointing to it. Add selinux_skb_sk() helper that is responsible to get back to the listener if skb is attached to a request socket, instead of duplicating the logic. Fixes: ca6fb0651883 ("tcp: attach SYNACK messages to request sockets instead of listener") Signed-off-by: Eric Dumazet Reported-by: Dmitry Vyukov Cc: Paul Moore Signed-off-by: David S. Miller --- security/selinux/hooks.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 26f4039d54b8..c9b2d5467477 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -4931,11 +4931,23 @@ static unsigned int selinux_ipv4_output(void *priv, return selinux_ip_output(skb, PF_INET); } +/* SYNACK messages might be attached to request sockets. + * To get back to sk_security, we need to look at the listener. + */ +static struct sock *selinux_skb_sk(const struct sk_buff *skb) +{ + struct sock *sk = skb->sk; + + if (sk && sk->sk_state == TCP_NEW_SYN_RECV) + sk = inet_reqsk(sk)->rsk_listener; + return sk; +} + static unsigned int selinux_ip_postroute_compat(struct sk_buff *skb, int ifindex, u16 family) { - struct sock *sk = skb->sk; + struct sock *sk = selinux_skb_sk(skb); struct sk_security_struct *sksec; struct common_audit_data ad; struct lsm_network_audit net = {0,}; @@ -4990,7 +5002,7 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, if (!secmark_active && !peerlbl_active) return NF_ACCEPT; - sk = skb->sk; + sk = selinux_skb_sk(skb); #ifdef CONFIG_XFRM /* If skb->dst->xfrm is non-NULL then the packet is undergoing an IPsec @@ -5035,8 +5047,6 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, u32 skb_sid; struct sk_security_struct *sksec; - if (sk->sk_state == TCP_NEW_SYN_RECV) - sk = inet_reqsk(sk)->rsk_listener; sksec = sk->sk_security; if (selinux_skb_peerlbl_sid(skb, family, &skb_sid)) return NF_DROP; -- cgit v1.2.3 From 71baba4b92dc1fa1bc461742c6ab1942ec6034e9 Mon Sep 17 00:00:00 2001 From: Mel Gorman Date: Fri, 6 Nov 2015 16:28:28 -0800 Subject: mm, page_alloc: rename __GFP_WAIT to __GFP_RECLAIM __GFP_WAIT was used to signal that the caller was in atomic context and could not sleep. Now it is possible to distinguish between true atomic context and callers that are not willing to sleep. The latter should clear __GFP_DIRECT_RECLAIM so kswapd will still wake. As clearing __GFP_WAIT behaves differently, there is a risk that people will clear the wrong flags. This patch renames __GFP_WAIT to __GFP_RECLAIM to clearly indicate what it does -- setting it allows all reclaim activity, clearing them prevents it. [akpm@linux-foundation.org: fix build] [akpm@linux-foundation.org: coding-style fixes] Signed-off-by: Mel Gorman Acked-by: Michal Hocko Acked-by: Vlastimil Babka Acked-by: Johannes Weiner Cc: Christoph Lameter Acked-by: David Rientjes Cc: Vitaly Wool Cc: Rik van Riel Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- security/integrity/ima/ima_crypto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index e24121afb2f2..6eb62936c672 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -126,7 +126,7 @@ static void *ima_alloc_pages(loff_t max_size, size_t *allocated_size, { void *ptr; int order = ima_maxorder; - gfp_t gfp_mask = __GFP_WAIT | __GFP_NOWARN | __GFP_NORETRY; + gfp_t gfp_mask = __GFP_RECLAIM | __GFP_NOWARN | __GFP_NORETRY; if (order) order = min(get_order(max_size), order); -- cgit v1.2.3 From 54abc686c2d111e98228943a7e1e51cc256e35d9 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Sun, 8 Nov 2015 10:54:07 -0800 Subject: net: add skb_to_full_sk() helper and use it in selinux_netlbl_skbuff_setsid() Generalize selinux_skb_sk() added in commit 212cd0895330 ("selinux: fix random read in selinux_ip_postroute_compat()") so that we can use it other contexts. Use it right away in selinux_netlbl_skbuff_setsid() Fixes: ca6fb0651883 ("tcp: attach SYNACK messages to request sockets instead of listener") Signed-off-by: Eric Dumazet Signed-off-by: David S. Miller --- security/selinux/hooks.c | 16 ++-------------- security/selinux/netlabel.c | 2 +- 2 files changed, 3 insertions(+), 15 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index c9b2d5467477..23307da15f1d 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -4931,23 +4931,11 @@ static unsigned int selinux_ipv4_output(void *priv, return selinux_ip_output(skb, PF_INET); } -/* SYNACK messages might be attached to request sockets. - * To get back to sk_security, we need to look at the listener. - */ -static struct sock *selinux_skb_sk(const struct sk_buff *skb) -{ - struct sock *sk = skb->sk; - - if (sk && sk->sk_state == TCP_NEW_SYN_RECV) - sk = inet_reqsk(sk)->rsk_listener; - return sk; -} - static unsigned int selinux_ip_postroute_compat(struct sk_buff *skb, int ifindex, u16 family) { - struct sock *sk = selinux_skb_sk(skb); + struct sock *sk = skb_to_full_sk(skb); struct sk_security_struct *sksec; struct common_audit_data ad; struct lsm_network_audit net = {0,}; @@ -5002,7 +4990,7 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, if (!secmark_active && !peerlbl_active) return NF_ACCEPT; - sk = selinux_skb_sk(skb); + sk = skb_to_full_sk(skb); #ifdef CONFIG_XFRM /* If skb->dst->xfrm is non-NULL then the packet is undergoing an IPsec diff --git a/security/selinux/netlabel.c b/security/selinux/netlabel.c index 0364120d1ec8..1f989a539fd4 100644 --- a/security/selinux/netlabel.c +++ b/security/selinux/netlabel.c @@ -245,7 +245,7 @@ int selinux_netlbl_skbuff_setsid(struct sk_buff *skb, /* if this is a locally generated packet check to see if it is already * being labeled by it's parent socket, if it is just exit */ - sk = skb->sk; + sk = skb_to_full_sk(skb); if (sk != NULL) { struct sk_security_struct *sksec = sk->sk_security; if (sksec->nlbl_state != NLBL_REQSKB) -- cgit v1.2.3 From 8827d90e29e664aa959817467a3da72041ca2269 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Sun, 8 Nov 2015 10:54:08 -0800 Subject: smack: use skb_to_full_sk() helper This module wants to access sk->sk_security, which is not available for request sockets. Fixes: ca6fb0651883 ("tcp: attach SYNACK messages to request sockets instead of listener") Signed-off-by: Eric Dumazet Signed-off-by: David S. Miller --- security/smack/smack_netfilter.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'security') diff --git a/security/smack/smack_netfilter.c b/security/smack/smack_netfilter.c index 6d1706c9777e..aa6bf1b22ec5 100644 --- a/security/smack/smack_netfilter.c +++ b/security/smack/smack_netfilter.c @@ -17,6 +17,7 @@ #include #include #include +#include #include "smack.h" #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE) @@ -25,11 +26,12 @@ static unsigned int smack_ipv6_output(void *priv, struct sk_buff *skb, const struct nf_hook_state *state) { + struct sock *sk = skb_to_full_sk(skb); struct socket_smack *ssp; struct smack_known *skp; - if (skb && skb->sk && skb->sk->sk_security) { - ssp = skb->sk->sk_security; + if (sk && sk->sk_security) { + ssp = sk->sk_security; skp = ssp->smk_out; skb->secmark = skp->smk_secid; } @@ -42,11 +44,12 @@ static unsigned int smack_ipv4_output(void *priv, struct sk_buff *skb, const struct nf_hook_state *state) { + struct sock *sk = skb_to_full_sk(skb); struct socket_smack *ssp; struct smack_known *skp; - if (skb && skb->sk && skb->sk->sk_security) { - ssp = skb->sk->sk_security; + if (sk && sk->sk_security) { + ssp = sk->sk_security; skp = ssp->smk_out; skb->secmark = skp->smk_secid; } -- cgit v1.2.3