tree 04f08b1e5bbb373cfa892346db5be8ab16342114
parent 1b8b31a2e6120b7b2bc99137c0ba1ae3e45dbd7d
author Dan Carpenter <dan.carpenter@oracle.com> 1598451259 -0400
committer Paul Moore <paul@paul-moore.com> 1598451548 -0400

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>