selinux: fix error handling bugs in security_load_policy()
authorDan Carpenter <dan.carpenter@oracle.com>
Wed, 26 Aug 2020 14:14:19 +0000 (10:14 -0400)
committerPaul Moore <paul@paul-moore.com>
Wed, 26 Aug 2020 14:19:08 +0000 (10:19 -0400)
commit0256b0aa8019d937a0bdce3584c6b8b47f618202
tree04f08b1e5bbb373cfa892346db5be8ab16342114
parent1b8b31a2e6120b7b2bc99137c0ba1ae3e45dbd7d
selinux: fix error handling bugs in security_load_policy()

There are a few bugs in the error handling for security_load_policy().

1) If the newpolicy->sidtab allocation fails then it leads to a NULL
   dereference.  Also the error code was not set to -ENOMEM on that
   path.
2) If policydb_read() failed then we call policydb_destroy() twice
   which meands we call kvfree(p->sym_val_to_name[i]) twice.
3) If policydb_load_isids() failed then we call sidtab_destroy() twice
   and that results in a double free in the sidtab_destroy_tree()
   function because entry.ptr_inner and entry.ptr_leaf are not set to
   NULL.

One thing that makes this code nice to deal with is that none of the
functions return partially allocated data.  In other words, the
policydb_read() either allocates everything successfully or it frees
all the data it allocates.  It never returns a mix of allocated and
not allocated data.

I re-wrote this to only free the successfully allocated data which
avoids the double frees.  I also re-ordered selinux_policy_free() so
it's in the reverse order of the allocation function.

Fixes: c7c556f1e81b ("selinux: refactor changing booleans")
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
[PM: partially merged by hand due to merge fuzz]
Signed-off-by: Paul Moore <paul@paul-moore.com>
security/selinux/ss/services.c