]> git.baikalelectronics.ru Git - kernel.git/commit
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)
commit05383e82e008582c98073b37f28900c91ba997f7
tree04f08b1e5bbb373cfa892346db5be8ab16342114
parentbb75f1077da40483a8488c39aaf3b5b51c07e825
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: 879a0f10dba1 ("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