From ffb733c65000ee701294f7b80c4eca2a5f335637 Mon Sep 17 00:00:00 2001 From: "paul.moore@hp.com" Date: Wed, 4 Oct 2006 11:46:31 -0400 Subject: NetLabel: fix a cache race condition Testing revealed a problem with the NetLabel cache where a cached entry could be freed while in use by the LSM layer causing an oops and other problems. This patch fixes that problem by introducing a reference counter to the cache entry so that it is only freed when it is no longer in use. Signed-off-by: Paul Moore Signed-off-by: James Morris --- include/net/netlabel.h | 62 ++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 47 insertions(+), 15 deletions(-) (limited to 'include') diff --git a/include/net/netlabel.h b/include/net/netlabel.h index c63a58058e2..113337c2795 100644 --- a/include/net/netlabel.h +++ b/include/net/netlabel.h @@ -34,6 +34,7 @@ #include #include #include +#include /* * NetLabel - A management interface for maintaining network packet label @@ -106,6 +107,7 @@ int netlbl_domhsh_remove(const char *domain, struct netlbl_audit *audit_info); /* LSM security attributes */ struct netlbl_lsm_cache { + atomic_t refcount; void (*free) (const void *data); void *data; }; @@ -117,7 +119,7 @@ struct netlbl_lsm_secattr { unsigned char *mls_cat; size_t mls_cat_len; - struct netlbl_lsm_cache cache; + struct netlbl_lsm_cache *cache; }; /* @@ -125,6 +127,43 @@ struct netlbl_lsm_secattr { */ +/** + * netlbl_secattr_cache_alloc - Allocate and initialize a secattr cache + * @flags: the memory allocation flags + * + * Description: + * Allocate and initialize a netlbl_lsm_cache structure. Returns a pointer + * on success, NULL on failure. + * + */ +static inline struct netlbl_lsm_cache *netlbl_secattr_cache_alloc(int flags) +{ + struct netlbl_lsm_cache *cache; + + cache = kzalloc(sizeof(*cache), flags); + if (cache) + atomic_set(&cache->refcount, 1); + return cache; +} + +/** + * netlbl_secattr_cache_free - Frees a netlbl_lsm_cache struct + * @cache: the struct to free + * + * Description: + * Frees @secattr including all of the internal buffers. + * + */ +static inline void netlbl_secattr_cache_free(struct netlbl_lsm_cache *cache) +{ + if (!atomic_dec_and_test(&cache->refcount)) + return; + + if (cache->free) + cache->free(cache->data); + kfree(cache); +} + /** * netlbl_secattr_init - Initialize a netlbl_lsm_secattr struct * @secattr: the struct to initialize @@ -143,20 +182,16 @@ static inline int netlbl_secattr_init(struct netlbl_lsm_secattr *secattr) /** * netlbl_secattr_destroy - Clears a netlbl_lsm_secattr struct * @secattr: the struct to clear - * @clear_cache: cache clear flag * * Description: * Destroys the @secattr struct, including freeing all of the internal buffers. - * If @clear_cache is true then free the cache fields, otherwise leave them - * intact. The struct must be reset with a call to netlbl_secattr_init() - * before reuse. + * The struct must be reset with a call to netlbl_secattr_init() before reuse. * */ -static inline void netlbl_secattr_destroy(struct netlbl_lsm_secattr *secattr, - u32 clear_cache) +static inline void netlbl_secattr_destroy(struct netlbl_lsm_secattr *secattr) { - if (clear_cache && secattr->cache.data != NULL && secattr->cache.free) - secattr->cache.free(secattr->cache.data); + if (secattr->cache) + netlbl_secattr_cache_free(secattr->cache); kfree(secattr->domain); kfree(secattr->mls_cat); } @@ -178,17 +213,14 @@ static inline struct netlbl_lsm_secattr *netlbl_secattr_alloc(int flags) /** * netlbl_secattr_free - Frees a netlbl_lsm_secattr struct * @secattr: the struct to free - * @clear_cache: cache clear flag * * Description: - * Frees @secattr including all of the internal buffers. If @clear_cache is - * true then free the cache fields, otherwise leave them intact. + * Frees @secattr including all of the internal buffers. * */ -static inline void netlbl_secattr_free(struct netlbl_lsm_secattr *secattr, - u32 clear_cache) +static inline void netlbl_secattr_free(struct netlbl_lsm_secattr *secattr) { - netlbl_secattr_destroy(secattr, clear_cache); + netlbl_secattr_destroy(secattr); kfree(secattr); } -- cgit v1.2.3 From 134b0fc544ba062498451611cb6f3e4454221b3d Mon Sep 17 00:00:00 2001 From: James Morris Date: Thu, 5 Oct 2006 15:42:27 -0500 Subject: IPsec: propagate security module errors up from flow_cache_lookup When a security module is loaded (in this case, SELinux), the security_xfrm_policy_lookup() hook can return an access denied permission (or other error). We were not handling that correctly, and in fact inverting the return logic and propagating a false "ok" back up to xfrm_lookup(), which then allowed packets to pass as if they were not associated with an xfrm policy. The way I was seeing the problem was when connecting via IPsec to a confined service on an SELinux box (vsftpd), which did not have the appropriate SELinux policy permissions to send packets via IPsec. The first SYNACK would be blocked, because of an uncached lookup via flow_cache_lookup(), which would fail to resolve an xfrm policy because the SELinux policy is checked at that point via the resolver. However, retransmitted SYNACKs would then find a cached flow entry when calling into flow_cache_lookup() with a null xfrm policy, which is interpreted by xfrm_lookup() as the packet not having any associated policy and similarly to the first case, allowing it to pass without transformation. The solution presented here is to first ensure that errno values are correctly propagated all the way back up through the various call chains from security_xfrm_policy_lookup(), and handled correctly. Then, flow_cache_lookup() is modified, so that if the policy resolver fails (typically a permission denied via the security module), the flow cache entry is killed rather than having a null policy assigned (which indicates that the packet can pass freely). This also forces any future lookups for the same flow to consult the security module (e.g. SELinux) for current security policy (rather than, say, caching the error on the flow cache entry). Signed-off-by: James Morris --- include/net/flow.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include') diff --git a/include/net/flow.h b/include/net/flow.h index ddf5f3ca172..3b44d72b27d 100644 --- a/include/net/flow.h +++ b/include/net/flow.h @@ -97,7 +97,7 @@ struct flowi { #define FLOW_DIR_FWD 2 struct sock; -typedef void (*flow_resolve_t)(struct flowi *key, u16 family, u8 dir, +typedef int (*flow_resolve_t)(struct flowi *key, u16 family, u8 dir, void **objp, atomic_t **obj_refp); extern void *flow_cache_lookup(struct flowi *key, u16 family, u8 dir, -- cgit v1.2.3 From 5b368e61c2bcb2666bb66e2acf1d6d85ba6f474d Mon Sep 17 00:00:00 2001 From: Venkat Yekkirala Date: Thu, 5 Oct 2006 15:42:18 -0500 Subject: IPsec: correct semantics for SELinux policy matching Currently when an IPSec policy rule doesn't specify a security context, it is assumed to be "unlabeled" by SELinux, and so the IPSec policy rule fails to match to a flow that it would otherwise match to, unless one has explicitly added an SELinux policy rule allowing the flow to "polmatch" to the "unlabeled" IPSec policy rules. In the absence of such an explicitly added SELinux policy rule, the IPSec policy rule fails to match and so the packet(s) flow in clear text without the otherwise applicable xfrm(s) applied. The above SELinux behavior violates the SELinux security notion of "deny by default" which should actually translate to "encrypt by default" in the above case. This was first reported by Evgeniy Polyakov and the way James Morris was seeing the problem was when connecting via IPsec to a confined service on an SELinux box (vsftpd), which did not have the appropriate SELinux policy permissions to send packets via IPsec. With this patch applied, SELinux "polmatching" of flows Vs. IPSec policy rules will only come into play when there's a explicit context specified for the IPSec policy rule (which also means there's corresponding SELinux policy allowing appropriate domains/flows to polmatch to this context). Secondly, when a security module is loaded (in this case, SELinux), the security_xfrm_policy_lookup() hook can return errors other than access denied, such as -EINVAL. We were not handling that correctly, and in fact inverting the return logic and propagating a false "ok" back up to xfrm_lookup(), which then allowed packets to pass as if they were not associated with an xfrm policy. The solution for this is to first ensure that errno values are correctly propagated all the way back up through the various call chains from security_xfrm_policy_lookup(), and handled correctly. Then, flow_cache_lookup() is modified, so that if the policy resolver fails (typically a permission denied via the security module), the flow cache entry is killed rather than having a null policy assigned (which indicates that the packet can pass freely). This also forces any future lookups for the same flow to consult the security module (e.g. SELinux) for current security policy (rather than, say, caching the error on the flow cache entry). This patch: Fix the selinux side of things. This makes sure SELinux polmatching of flow contexts to IPSec policy rules comes into play only when an explicit context is associated with the IPSec policy rule. Also, this no longer defaults the context of a socket policy to the context of the socket since the "no explicit context" case is now handled properly. Signed-off-by: Venkat Yekkirala Signed-off-by: James Morris --- include/linux/security.h | 24 +++++++++--------------- include/net/xfrm.h | 3 ++- 2 files changed, 11 insertions(+), 16 deletions(-) (limited to 'include') diff --git a/include/linux/security.h b/include/linux/security.h index 9b5fea81f55..b200b9856f3 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -882,7 +882,8 @@ struct request_sock; * Check permission when a flow selects a xfrm_policy for processing * XFRMs on a packet. The hook is called when selecting either a * per-socket policy or a generic xfrm policy. - * Return 0 if permission is granted. + * Return 0 if permission is granted, -ESRCH otherwise, or -errno + * on other errors. * @xfrm_state_pol_flow_match: * @x contains the state to match. * @xp contains the policy to check for a match. @@ -891,6 +892,7 @@ struct request_sock; * @xfrm_flow_state_match: * @fl contains the flow key to match. * @xfrm points to the xfrm_state to match. + * @xp points to the xfrm_policy to match. * Return 1 if there is a match. * @xfrm_decode_session: * @skb points to skb to decode. @@ -1388,7 +1390,8 @@ struct security_operations { int (*xfrm_policy_lookup)(struct xfrm_policy *xp, u32 fl_secid, u8 dir); int (*xfrm_state_pol_flow_match)(struct xfrm_state *x, struct xfrm_policy *xp, struct flowi *fl); - int (*xfrm_flow_state_match)(struct flowi *fl, struct xfrm_state *xfrm); + int (*xfrm_flow_state_match)(struct flowi *fl, struct xfrm_state *xfrm, + struct xfrm_policy *xp); int (*xfrm_decode_session)(struct sk_buff *skb, u32 *secid, int ckall); #endif /* CONFIG_SECURITY_NETWORK_XFRM */ @@ -3120,11 +3123,6 @@ static inline int security_xfrm_policy_alloc(struct xfrm_policy *xp, struct xfrm return security_ops->xfrm_policy_alloc_security(xp, sec_ctx, NULL); } -static inline int security_xfrm_sock_policy_alloc(struct xfrm_policy *xp, struct sock *sk) -{ - return security_ops->xfrm_policy_alloc_security(xp, NULL, sk); -} - static inline int security_xfrm_policy_clone(struct xfrm_policy *old, struct xfrm_policy *new) { return security_ops->xfrm_policy_clone_security(old, new); @@ -3175,9 +3173,10 @@ static inline int security_xfrm_state_pol_flow_match(struct xfrm_state *x, return security_ops->xfrm_state_pol_flow_match(x, xp, fl); } -static inline int security_xfrm_flow_state_match(struct flowi *fl, struct xfrm_state *xfrm) +static inline int security_xfrm_flow_state_match(struct flowi *fl, + struct xfrm_state *xfrm, struct xfrm_policy *xp) { - return security_ops->xfrm_flow_state_match(fl, xfrm); + return security_ops->xfrm_flow_state_match(fl, xfrm, xp); } static inline int security_xfrm_decode_session(struct sk_buff *skb, u32 *secid) @@ -3197,11 +3196,6 @@ static inline int security_xfrm_policy_alloc(struct xfrm_policy *xp, struct xfrm return 0; } -static inline int security_xfrm_sock_policy_alloc(struct xfrm_policy *xp, struct sock *sk) -{ - return 0; -} - static inline int security_xfrm_policy_clone(struct xfrm_policy *old, struct xfrm_policy *new) { return 0; @@ -3249,7 +3243,7 @@ static inline int security_xfrm_state_pol_flow_match(struct xfrm_state *x, } static inline int security_xfrm_flow_state_match(struct flowi *fl, - struct xfrm_state *xfrm) + struct xfrm_state *xfrm, struct xfrm_policy *xp) { return 1; } diff --git a/include/net/xfrm.h b/include/net/xfrm.h index 1e2a4ddec96..737fdb2ee8a 100644 --- a/include/net/xfrm.h +++ b/include/net/xfrm.h @@ -995,7 +995,8 @@ struct xfrm_state * xfrm_find_acq(u8 mode, u32 reqid, u8 proto, int create, unsigned short family); extern void xfrm_policy_flush(u8 type); extern int xfrm_sk_policy_insert(struct sock *sk, int dir, struct xfrm_policy *pol); -extern int xfrm_bundle_ok(struct xfrm_dst *xdst, struct flowi *fl, int family, int strict); +extern int xfrm_bundle_ok(struct xfrm_policy *pol, struct xfrm_dst *xdst, + struct flowi *fl, int family, int strict); extern void xfrm_init_pmtu(struct dst_entry *dst); extern wait_queue_head_t km_waitq; -- cgit v1.2.3 From 331c4ee7faa4ee1e1404c872a139784753100498 Mon Sep 17 00:00:00 2001 From: Vlad Yasevich Date: Mon, 9 Oct 2006 21:34:04 -0700 Subject: [SCTP]: Fix receive buffer accounting. When doing receiver buffer accounting, we always used skb->truesize. This is problematic when processing bundled DATA chunks because for every DATA chunk that could be small part of one large skb, we would charge the size of the entire skb. The new approach is to store the size of the DATA chunk we are accounting for in the sctp_ulpevent structure and use that stored value for accounting. Signed-off-by: Vlad Yasevich Signed-off-by: Sridhar Samudrala Signed-off-by: David S. Miller --- include/net/sctp/sctp.h | 14 ++++++++++++++ include/net/sctp/ulpevent.h | 1 + 2 files changed, 15 insertions(+) (limited to 'include') diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h index ee68a312407..764e3af5be9 100644 --- a/include/net/sctp/sctp.h +++ b/include/net/sctp/sctp.h @@ -139,6 +139,7 @@ int sctp_inet_listen(struct socket *sock, int backlog); void sctp_write_space(struct sock *sk); unsigned int sctp_poll(struct file *file, struct socket *sock, poll_table *wait); +void sctp_sock_rfree(struct sk_buff *skb); /* * sctp/primitive.c @@ -444,6 +445,19 @@ static inline struct list_head *sctp_list_dequeue(struct list_head *list) return result; } +/* SCTP version of skb_set_owner_r. We need this one because + * of the way we have to do receive buffer accounting on bundled + * chunks. + */ +static inline void sctp_skb_set_owner_r(struct sk_buff *skb, struct sock *sk) +{ + struct sctp_ulpevent *event = sctp_skb2event(skb); + + skb->sk = sk; + skb->destructor = sctp_sock_rfree; + atomic_add(event->rmem_len, &sk->sk_rmem_alloc); +} + /* Tests if the list has one and only one entry. */ static inline int sctp_list_single_entry(struct list_head *head) { diff --git a/include/net/sctp/ulpevent.h b/include/net/sctp/ulpevent.h index 6c40cfc4832..1a4ddc1ec7d 100644 --- a/include/net/sctp/ulpevent.h +++ b/include/net/sctp/ulpevent.h @@ -63,6 +63,7 @@ struct sctp_ulpevent { __u32 cumtsn; int msg_flags; int iif; + unsigned int rmem_len; }; /* Retrieve the skb this event sits inside of. */ -- cgit v1.2.3 From 42b6785eeb40fe3e9dab9981b6e3231a77c7c2f6 Mon Sep 17 00:00:00 2001 From: YOSHIFUJI Hideaki Date: Tue, 10 Oct 2006 19:42:09 -0700 Subject: [NET]: Introduce protocol-specific destructor for time-wait sockets. Signed-off-by: YOSHIFUJI Hideaki Signed-off-by: David S. Miller --- include/net/inet_timewait_sock.h | 1 + include/net/timewait_sock.h | 7 +++++++ 2 files changed, 8 insertions(+) (limited to 'include') diff --git a/include/net/inet_timewait_sock.h b/include/net/inet_timewait_sock.h index 6d14c22a00c..5f48748fe01 100644 --- a/include/net/inet_timewait_sock.h +++ b/include/net/inet_timewait_sock.h @@ -196,6 +196,7 @@ static inline void inet_twsk_put(struct inet_timewait_sock *tw) { if (atomic_dec_and_test(&tw->tw_refcnt)) { struct module *owner = tw->tw_prot->owner; + twsk_destructor((struct sock *)tw); #ifdef SOCK_REFCNT_DEBUG printk(KERN_DEBUG "%s timewait_sock %p released\n", tw->tw_prot->name, tw); diff --git a/include/net/timewait_sock.h b/include/net/timewait_sock.h index 2544281e1d5..be293d795e3 100644 --- a/include/net/timewait_sock.h +++ b/include/net/timewait_sock.h @@ -19,6 +19,7 @@ struct timewait_sock_ops { unsigned int twsk_obj_size; int (*twsk_unique)(struct sock *sk, struct sock *sktw, void *twp); + void (*twsk_destructor)(struct sock *sk); }; static inline int twsk_unique(struct sock *sk, struct sock *sktw, void *twp) @@ -28,4 +29,10 @@ static inline int twsk_unique(struct sock *sk, struct sock *sktw, void *twp) return 0; } +static inline void twsk_destructor(struct sock *sk) +{ + if (sk->sk_prot->twsk_prot->twsk_destructor != NULL) + sk->sk_prot->twsk_prot->twsk_destructor(sk); +} + #endif /* _TIMEWAIT_SOCK_H */ -- cgit v1.2.3