[PATCH 28/32] selinux: Use mem_to_flex_dup() with xfrm and sidtab

Kees Cook
2022-05-04T01:58:32Z

As part of the work to perform bounds checking on all memcpy() uses,
replace the open-coded a deserialization of bytes out of memory into a
trailing flexible array by using a flex_array.h helper to perform the
allocation, bounds checking, and copying:

struct xfrm_sec_ctx
struct sidtab_str_cache

Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Stephen Smalley <stephen.smalley.work@gmail.com>
Cc: Eric Paris <eparis@parisplace.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Xiu Jianfeng <xiujianfeng@huawei.com>
Cc: "Christian Göttsche" <cgzones@googlemail.com>
Cc: netdev@vger.kernel.org
Cc: selinux@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
include/uapi/linux/xfrm.h | 4 ++--
security/selinux/ss/sidtab.c | 9 +++------
security/selinux/xfrm.c | 7 ++-----
3 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
index 65e13a099b1a..4a6fa2beff6a 100644
--- a/include/uapi/linux/xfrm.h
+++ b/include/uapi/linux/xfrm.h
@@ -31,9 +31,9 @@ struct xfrm_id {
struct xfrm_sec_ctx {
__u8 ctx_doi;
__u8 ctx_alg;
- __u16 ctx_len;
+ __DECLARE_FLEX_ARRAY_ELEMENTS_COUNT(__u16, ctx_len);
__u32 ctx_sid;
- char ctx_str[0];
+ __DECLARE_FLEX_ARRAY_ELEMENTS(char, ctx_str);
};

/* Security Context Domains of Interpretation */
diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c
index a54b8652bfb5..a9d434e8cff7 100644
--- a/security/selinux/ss/sidtab.c
+++ b/security/selinux/ss/sidtab.c
@@ -23,8 +23,8 @@ struct sidtab_str_cache {
struct rcu_head rcu_member;
struct list_head lru_member;
struct sidtab_entry *parent;
- u32 len;
- char str[];
+ DECLARE_FLEX_ARRAY_ELEMENTS_COUNT(u32, len);
+ DECLARE_FLEX_ARRAY_ELEMENTS(char, str);
};

#define index_to_sid(index) ((index) + SECINITSID_NUM + 1)
@@ -570,8 +570,7 @@ void sidtab_sid2str_put(struct sidtab *s, struct sidtab_entry *entry,
goto out_unlock;
}

- cache = kmalloc(struct_size(cache, str, str_len), GFP_ATOMIC);
- if (!cache)
+ if (mem_to_flex_dup(&cache, str, str_len, GFP_ATOMIC))
goto out_unlock;

if (s->cache_free_slots == 0) {
@@ -584,8 +583,6 @@ void sidtab_sid2str_put(struct sidtab *s, struct sidtab_entry *entry,
s->cache_free_slots--;
}
cache->parent = entry;
- cache->len = str_len;
- memcpy(cache->str, str, str_len);
list_add(&cache->lru_member, &s->cache_lru_list);

rcu_assign_pointer(entry->cache, cache);
diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c
index c576832febc6..bc7a54bf8f0d 100644
--- a/security/selinux/xfrm.c
+++ b/security/selinux/xfrm.c
@@ -345,7 +345,7 @@ int selinux_xfrm_state_alloc_acquire(struct xfrm_state *x,
struct xfrm_sec_ctx *polsec, u32 secid)
{
int rc;
- struct xfrm_sec_ctx *ctx;
+ struct xfrm_sec_ctx *ctx = NULL;
char *ctx_str = NULL;
u32 str_len;

@@ -360,8 +360,7 @@ int selinux_xfrm_state_alloc_acquire(struct xfrm_state *x,
if (rc)
return rc;

- ctx = kmalloc(struct_size(ctx, ctx_str, str_len), GFP_ATOMIC);
- if (!ctx) {
+ if (mem_to_flex_dup(&ctx, ctx_str, str_len, GFP_ATOMIC)) {
rc = -ENOMEM;
goto out;
}
@@ -369,8 +368,6 @@ int selinux_xfrm_state_alloc_acquire(struct xfrm_state *x,
ctx->ctx_doi = XFRM_SC_DOI_LSM;
ctx->ctx_alg = XFRM_SC_ALG_SELINUX;
ctx->ctx_sid = secid;
- ctx->ctx_len = str_len;
- memcpy(ctx->ctx_str, ctx_str, str_len);

x->security = ctx;
atomic_inc(&selinux_xfrm_refcount);
--
2.32.0