Skip to content

Commit

Permalink
selinux: check length fields in policies
Browse files Browse the repository at this point in the history
In multiple places the binary policy announces how many items of some
kind are to be expected next.  Before reading them the kernel already
allocates enough memory for that announced size.  Validate that the
remaining input size can actually fit the announced items, to avoid OOM
issues on malformed binary policies.

Signed-off-by: Christian Göttsche <[email protected]>
---
v3:
  - fix error branch by returning directly instead of jumping to goto
    label, see
      https://lore.kernel.org/all/[email protected]/
  - rename to size_check()
  - add comments for magic values
  • Loading branch information
cgzones committed Jan 8, 2025
1 parent 01238b2 commit 9190cd7
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 0 deletions.
5 changes: 5 additions & 0 deletions security/selinux/ss/avtab.c
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,11 @@ int avtab_read(struct avtab *a, struct policy_file *fp, struct policydb *pol)
goto bad;
}

/* avtab_read_item() reads at least 96 bytes for any valid entry */
rc = size_check(3 * sizeof(u32), nel, fp);
if (rc)
goto bad;

rc = avtab_alloc(a, nel);
if (rc)
goto bad;
Expand Down
17 changes: 17 additions & 0 deletions security/selinux/ss/conditional.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

#include "security.h"
#include "conditional.h"
#include "policydb.h"
#include "services.h"

/*
Expand Down Expand Up @@ -329,6 +330,11 @@ static int cond_read_av_list(struct policydb *p, struct policy_file *fp,
if (len == 0)
return 0;

/* avtab_read_item() reads at least 96 bytes for any valid entry */
rc = size_check(3 * sizeof(u32), len, fp);
if (rc)
return rc;

list->nodes = kcalloc(len, sizeof(*list->nodes), GFP_KERNEL);
if (!list->nodes)
return -ENOMEM;
Expand Down Expand Up @@ -379,6 +385,12 @@ static int cond_read_node(struct policydb *p, struct cond_node *node, struct pol

/* expr */
len = le32_to_cpu(buf[1]);

/* we will read 64 bytes per node */
rc = size_check(2 * sizeof(u32), len, fp);
if (rc)
return rc;

node->expr.nodes = kcalloc(len, sizeof(*node->expr.nodes), GFP_KERNEL);
if (!node->expr.nodes)
return -ENOMEM;
Expand Down Expand Up @@ -417,6 +429,11 @@ int cond_read_list(struct policydb *p, struct policy_file *fp)

len = le32_to_cpu(buf[0]);

/* cond_read_node() reads at least 128 bytes for any valid node */
rc = size_check(4 * sizeof(u32), len, fp);
if (rc)
return rc;

p->cond_list = kcalloc(len, sizeof(*p->cond_list), GFP_KERNEL);
if (!p->cond_list)
return -ENOMEM;
Expand Down
32 changes: 32 additions & 0 deletions security/selinux/ss/policydb.c
Original file line number Diff line number Diff line change
Expand Up @@ -1100,6 +1100,9 @@ int str_read(char **strp, gfp_t flags, struct policy_file *fp, u32 len)
if ((len == 0) || (len == (u32)-1))
return -EINVAL;

if (size_check(sizeof(char), len, fp))
return -EINVAL;

str = kmalloc(len + 1, flags | __GFP_NOWARN);
if (!str)
return -ENOMEM;
Expand Down Expand Up @@ -1174,6 +1177,11 @@ static int common_read(struct policydb *p, struct symtab *s, struct policy_file
if (nel > SEL_VEC_MAX)
goto bad;

/* perm_read() reads at least 64 bytes for any valid permission */
rc = size_check(2 * sizeof(u32), nel, fp);
if (rc)
goto bad;

rc = symtab_init(&comdatum->permissions, nel);
if (rc)
goto bad;
Expand Down Expand Up @@ -1348,6 +1356,11 @@ static int class_read(struct policydb *p, struct symtab *s, struct policy_file *
goto bad;
cladatum->value = val;

/* perm_read() reads at least 64 bytes for any valid permission */
rc = size_check(2 * sizeof(u32), nel, fp);
if (rc)
goto bad;

rc = symtab_init(&cladatum->permissions, nel);
if (rc)
goto bad;
Expand Down Expand Up @@ -1921,6 +1934,13 @@ static int range_read(struct policydb *p, struct policy_file *fp)

nel = le32_to_cpu(buf[0]);

/* we read at least 64 bytes and mls_read_range_helper() 32 bytes
* for any valid range-transition
*/
rc = size_check(3 * sizeof(u32), nel, fp);
if (rc)
return rc;

rc = hashtab_init(&p->range_tr, nel);
if (rc)
return rc;
Expand Down Expand Up @@ -2689,6 +2709,13 @@ int policydb_read(struct policydb *p, struct policy_file *fp)
nprim = le32_to_cpu(buf[0]);
nel = le32_to_cpu(buf[1]);

/* every read_f() implementation reads at least 128 bytes
* for any valid entry
*/
rc = size_check(4 * sizeof(u32), nel, fp);
if (rc)
goto out;

rc = symtab_init(&p->symtab[i], nel);
if (rc)
goto out;
Expand Down Expand Up @@ -2730,6 +2757,11 @@ int policydb_read(struct policydb *p, struct policy_file *fp)
goto bad;
nel = le32_to_cpu(buf[0]);

/* we read at least 96 bytes for any valid role-transition */
rc = size_check(3 * sizeof(u32), nel, fp);
if (rc)
goto bad;

rc = hashtab_init(&p->role_tr, nel);
if (rc)
goto bad;
Expand Down
13 changes: 13 additions & 0 deletions security/selinux/ss/policydb.h
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,19 @@ struct policy_data {
struct policy_file *fp;
};

static inline int size_check(size_t bytes, size_t num, const struct policy_file *fp)
{
size_t len;

if (unlikely(check_mul_overflow(bytes, num, &len)))
return -EINVAL;

if (unlikely(len > fp->len))
return -EINVAL;

return 0;
}

static inline int next_entry(void *buf, struct policy_file *fp, size_t bytes)
{
if (bytes > fp->len)
Expand Down

0 comments on commit 9190cd7

Please sign in to comment.