Commit ffb733c6 authored by paul.moore@hp.com's avatar paul.moore@hp.com Committed by David S. Miller

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: default avatarPaul Moore <paul.moore@hp.com>
Signed-off-by: default avatarJames Morris <jmorris@namei.org>
parent c25d5180
...@@ -34,6 +34,7 @@ ...@@ -34,6 +34,7 @@
#include <linux/net.h> #include <linux/net.h>
#include <linux/skbuff.h> #include <linux/skbuff.h>
#include <net/netlink.h> #include <net/netlink.h>
#include <asm/atomic.h>
/* /*
* NetLabel - A management interface for maintaining network packet label * 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); ...@@ -106,6 +107,7 @@ int netlbl_domhsh_remove(const char *domain, struct netlbl_audit *audit_info);
/* LSM security attributes */ /* LSM security attributes */
struct netlbl_lsm_cache { struct netlbl_lsm_cache {
atomic_t refcount;
void (*free) (const void *data); void (*free) (const void *data);
void *data; void *data;
}; };
...@@ -117,7 +119,7 @@ struct netlbl_lsm_secattr { ...@@ -117,7 +119,7 @@ struct netlbl_lsm_secattr {
unsigned char *mls_cat; unsigned char *mls_cat;
size_t mls_cat_len; size_t mls_cat_len;
struct netlbl_lsm_cache cache; struct netlbl_lsm_cache *cache;
}; };
/* /*
...@@ -125,6 +127,43 @@ struct netlbl_lsm_secattr { ...@@ -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 * netlbl_secattr_init - Initialize a netlbl_lsm_secattr struct
* @secattr: the struct to initialize * @secattr: the struct to initialize
...@@ -143,20 +182,16 @@ static inline int netlbl_secattr_init(struct netlbl_lsm_secattr *secattr) ...@@ -143,20 +182,16 @@ static inline int netlbl_secattr_init(struct netlbl_lsm_secattr *secattr)
/** /**
* netlbl_secattr_destroy - Clears a netlbl_lsm_secattr struct * netlbl_secattr_destroy - Clears a netlbl_lsm_secattr struct
* @secattr: the struct to clear * @secattr: the struct to clear
* @clear_cache: cache clear flag
* *
* Description: * Description:
* Destroys the @secattr struct, including freeing all of the internal buffers. * Destroys the @secattr struct, including freeing all of the internal buffers.
* If @clear_cache is true then free the cache fields, otherwise leave them * The struct must be reset with a call to netlbl_secattr_init() before reuse.
* intact. 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, static inline void netlbl_secattr_destroy(struct netlbl_lsm_secattr *secattr)
u32 clear_cache)
{ {
if (clear_cache && secattr->cache.data != NULL && secattr->cache.free) if (secattr->cache)
secattr->cache.free(secattr->cache.data); netlbl_secattr_cache_free(secattr->cache);
kfree(secattr->domain); kfree(secattr->domain);
kfree(secattr->mls_cat); kfree(secattr->mls_cat);
} }
...@@ -178,17 +213,14 @@ static inline struct netlbl_lsm_secattr *netlbl_secattr_alloc(int flags) ...@@ -178,17 +213,14 @@ static inline struct netlbl_lsm_secattr *netlbl_secattr_alloc(int flags)
/** /**
* netlbl_secattr_free - Frees a netlbl_lsm_secattr struct * netlbl_secattr_free - Frees a netlbl_lsm_secattr struct
* @secattr: the struct to free * @secattr: the struct to free
* @clear_cache: cache clear flag
* *
* Description: * Description:
* Frees @secattr including all of the internal buffers. If @clear_cache is * Frees @secattr including all of the internal buffers.
* true then free the cache fields, otherwise leave them intact.
* *
*/ */
static inline void netlbl_secattr_free(struct netlbl_lsm_secattr *secattr, static inline void netlbl_secattr_free(struct netlbl_lsm_secattr *secattr)
u32 clear_cache)
{ {
netlbl_secattr_destroy(secattr, clear_cache); netlbl_secattr_destroy(secattr);
kfree(secattr); kfree(secattr);
} }
......
...@@ -43,6 +43,7 @@ ...@@ -43,6 +43,7 @@
#include <net/tcp.h> #include <net/tcp.h>
#include <net/netlabel.h> #include <net/netlabel.h>
#include <net/cipso_ipv4.h> #include <net/cipso_ipv4.h>
#include <asm/atomic.h>
#include <asm/bug.h> #include <asm/bug.h>
struct cipso_v4_domhsh_entry { struct cipso_v4_domhsh_entry {
...@@ -79,7 +80,7 @@ struct cipso_v4_map_cache_entry { ...@@ -79,7 +80,7 @@ struct cipso_v4_map_cache_entry {
unsigned char *key; unsigned char *key;
size_t key_len; size_t key_len;
struct netlbl_lsm_cache lsm_data; struct netlbl_lsm_cache *lsm_data;
u32 activity; u32 activity;
struct list_head list; struct list_head list;
...@@ -188,13 +189,14 @@ static void cipso_v4_doi_domhsh_free(struct rcu_head *entry) ...@@ -188,13 +189,14 @@ static void cipso_v4_doi_domhsh_free(struct rcu_head *entry)
* @entry: the entry to free * @entry: the entry to free
* *
* Description: * Description:
* This function frees the memory associated with a cache entry. * This function frees the memory associated with a cache entry including the
* LSM cache data if there are no longer any users, i.e. reference count == 0.
* *
*/ */
static void cipso_v4_cache_entry_free(struct cipso_v4_map_cache_entry *entry) static void cipso_v4_cache_entry_free(struct cipso_v4_map_cache_entry *entry)
{ {
if (entry->lsm_data.free) if (entry->lsm_data)
entry->lsm_data.free(entry->lsm_data.data); netlbl_secattr_cache_free(entry->lsm_data);
kfree(entry->key); kfree(entry->key);
kfree(entry); kfree(entry);
} }
...@@ -315,8 +317,8 @@ static int cipso_v4_cache_check(const unsigned char *key, ...@@ -315,8 +317,8 @@ static int cipso_v4_cache_check(const unsigned char *key,
entry->key_len == key_len && entry->key_len == key_len &&
memcmp(entry->key, key, key_len) == 0) { memcmp(entry->key, key, key_len) == 0) {
entry->activity += 1; entry->activity += 1;
secattr->cache.free = entry->lsm_data.free; atomic_inc(&entry->lsm_data->refcount);
secattr->cache.data = entry->lsm_data.data; secattr->cache = entry->lsm_data;
if (prev_entry == NULL) { if (prev_entry == NULL) {
spin_unlock_bh(&cipso_v4_cache[bkt].lock); spin_unlock_bh(&cipso_v4_cache[bkt].lock);
return 0; return 0;
...@@ -383,8 +385,8 @@ int cipso_v4_cache_add(const struct sk_buff *skb, ...@@ -383,8 +385,8 @@ int cipso_v4_cache_add(const struct sk_buff *skb,
memcpy(entry->key, cipso_ptr, cipso_ptr_len); memcpy(entry->key, cipso_ptr, cipso_ptr_len);
entry->key_len = cipso_ptr_len; entry->key_len = cipso_ptr_len;
entry->hash = cipso_v4_map_cache_hash(cipso_ptr, cipso_ptr_len); entry->hash = cipso_v4_map_cache_hash(cipso_ptr, cipso_ptr_len);
entry->lsm_data.free = secattr->cache.free; atomic_inc(&secattr->cache->refcount);
entry->lsm_data.data = secattr->cache.data; entry->lsm_data = secattr->cache;
bkt = entry->hash & (CIPSO_V4_CACHE_BUCKETBITS - 1); bkt = entry->hash & (CIPSO_V4_CACHE_BUCKETBITS - 1);
spin_lock_bh(&cipso_v4_cache[bkt].lock); spin_lock_bh(&cipso_v4_cache[bkt].lock);
......
...@@ -200,7 +200,7 @@ void netlbl_cache_invalidate(void) ...@@ -200,7 +200,7 @@ void netlbl_cache_invalidate(void)
int netlbl_cache_add(const struct sk_buff *skb, int netlbl_cache_add(const struct sk_buff *skb,
const struct netlbl_lsm_secattr *secattr) const struct netlbl_lsm_secattr *secattr)
{ {
if (secattr->cache.data == NULL) if (secattr->cache == NULL)
return -ENOMSG; return -ENOMSG;
if (CIPSO_V4_OPTEXIST(skb)) if (CIPSO_V4_OPTEXIST(skb))
......
...@@ -2172,7 +2172,12 @@ struct netlbl_cache { ...@@ -2172,7 +2172,12 @@ struct netlbl_cache {
*/ */
static void selinux_netlbl_cache_free(const void *data) static void selinux_netlbl_cache_free(const void *data)
{ {
struct netlbl_cache *cache = NETLBL_CACHE(data); struct netlbl_cache *cache;
if (data == NULL)
return;
cache = NETLBL_CACHE(data);
switch (cache->type) { switch (cache->type) {
case NETLBL_CACHE_T_MLS: case NETLBL_CACHE_T_MLS:
ebitmap_destroy(&cache->data.mls_label.level[0].cat); ebitmap_destroy(&cache->data.mls_label.level[0].cat);
...@@ -2197,17 +2202,20 @@ static void selinux_netlbl_cache_add(struct sk_buff *skb, struct context *ctx) ...@@ -2197,17 +2202,20 @@ static void selinux_netlbl_cache_add(struct sk_buff *skb, struct context *ctx)
struct netlbl_lsm_secattr secattr; struct netlbl_lsm_secattr secattr;
netlbl_secattr_init(&secattr); netlbl_secattr_init(&secattr);
secattr.cache = netlbl_secattr_cache_alloc(GFP_ATOMIC);
if (secattr.cache == NULL)
goto netlbl_cache_add_return;
cache = kzalloc(sizeof(*cache), GFP_ATOMIC); cache = kzalloc(sizeof(*cache), GFP_ATOMIC);
if (cache == NULL) if (cache == NULL)
goto netlbl_cache_add_failure; goto netlbl_cache_add_return;
secattr.cache.free = selinux_netlbl_cache_free; secattr.cache->free = selinux_netlbl_cache_free;
secattr.cache.data = (void *)cache; secattr.cache->data = (void *)cache;
cache->type = NETLBL_CACHE_T_MLS; cache->type = NETLBL_CACHE_T_MLS;
if (ebitmap_cpy(&cache->data.mls_label.level[0].cat, if (ebitmap_cpy(&cache->data.mls_label.level[0].cat,
&ctx->range.level[0].cat) != 0) &ctx->range.level[0].cat) != 0)
goto netlbl_cache_add_failure; goto netlbl_cache_add_return;
cache->data.mls_label.level[1].cat.highbit = cache->data.mls_label.level[1].cat.highbit =
cache->data.mls_label.level[0].cat.highbit; cache->data.mls_label.level[0].cat.highbit;
cache->data.mls_label.level[1].cat.node = cache->data.mls_label.level[1].cat.node =
...@@ -2215,13 +2223,10 @@ static void selinux_netlbl_cache_add(struct sk_buff *skb, struct context *ctx) ...@@ -2215,13 +2223,10 @@ static void selinux_netlbl_cache_add(struct sk_buff *skb, struct context *ctx)
cache->data.mls_label.level[0].sens = ctx->range.level[0].sens; cache->data.mls_label.level[0].sens = ctx->range.level[0].sens;
cache->data.mls_label.level[1].sens = ctx->range.level[0].sens; cache->data.mls_label.level[1].sens = ctx->range.level[0].sens;
if (netlbl_cache_add(skb, &secattr) != 0) netlbl_cache_add(skb, &secattr);
goto netlbl_cache_add_failure;
return;
netlbl_cache_add_failure: netlbl_cache_add_return:
netlbl_secattr_destroy(&secattr, 1); netlbl_secattr_destroy(&secattr);
} }
/** /**
...@@ -2263,8 +2268,8 @@ static int selinux_netlbl_secattr_to_sid(struct sk_buff *skb, ...@@ -2263,8 +2268,8 @@ static int selinux_netlbl_secattr_to_sid(struct sk_buff *skb,
POLICY_RDLOCK; POLICY_RDLOCK;
if (secattr->cache.data) { if (secattr->cache) {
cache = NETLBL_CACHE(secattr->cache.data); cache = NETLBL_CACHE(secattr->cache->data);
switch (cache->type) { switch (cache->type) {
case NETLBL_CACHE_T_SID: case NETLBL_CACHE_T_SID:
*sid = cache->data.sid; *sid = cache->data.sid;
...@@ -2369,7 +2374,7 @@ static int selinux_netlbl_skbuff_getsid(struct sk_buff *skb, ...@@ -2369,7 +2374,7 @@ static int selinux_netlbl_skbuff_getsid(struct sk_buff *skb,
&secattr, &secattr,
base_sid, base_sid,
sid); sid);
netlbl_secattr_destroy(&secattr, 0); netlbl_secattr_destroy(&secattr);
return rc; return rc;
} }
...@@ -2415,7 +2420,7 @@ static int selinux_netlbl_socket_setsid(struct socket *sock, u32 sid) ...@@ -2415,7 +2420,7 @@ static int selinux_netlbl_socket_setsid(struct socket *sock, u32 sid)
if (rc == 0) if (rc == 0)
sksec->nlbl_state = NLBL_LABELED; sksec->nlbl_state = NLBL_LABELED;
netlbl_secattr_destroy(&secattr, 0); netlbl_secattr_destroy(&secattr);
netlbl_socket_setsid_return: netlbl_socket_setsid_return:
POLICY_RDUNLOCK; POLICY_RDUNLOCK;
...@@ -2517,7 +2522,7 @@ void selinux_netlbl_sock_graft(struct sock *sk, struct socket *sock) ...@@ -2517,7 +2522,7 @@ void selinux_netlbl_sock_graft(struct sock *sk, struct socket *sock)
sksec->sid, sksec->sid,
&nlbl_peer_sid) == 0) &nlbl_peer_sid) == 0)
sksec->peer_sid = nlbl_peer_sid; sksec->peer_sid = nlbl_peer_sid;
netlbl_secattr_destroy(&secattr, 0); netlbl_secattr_destroy(&secattr);
sksec->nlbl_state = NLBL_REQUIRE; sksec->nlbl_state = NLBL_REQUIRE;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment