lkml.org 
[lkml]   [2019]   [Mar]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] selinux: fix NULL dereference in policydb_destroy()
From
Date
On 3/17/19 9:46 AM, Ondrej Mosnacek wrote:
> The conversion to kvmalloc() forgot to account for the possibility that
> p->type_attr_map_array might be null in policydb_destroy().
>
> Fix this by destroying its contents only if it is not NULL.
>
> Also make sure ebitmap_init() is called on all entries before
> policydb_destroy() can be called. Right now this is a no-op, because
> both kvcalloc() and ebitmap_init() just zero out the whole struct, but
> let's rather not rely on a specific implementation.
>
> Reported-by: syzbot+a57b2aff60832666fc28@syzkaller.appspotmail.com
> Fixes: acdf52d97f82 ("selinux: convert to kvmalloc")
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>

Acked-by: Stephen Smalley <sds@tycho.nsa.gov>

> ---
> security/selinux/ss/policydb.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> NOTE: This applies directly on top of current Linus' tree, since the
> problematic commit is not present in the selinux/stable-5.1 branch.
>
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index 6b576e588725..daecdfb15a9c 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -828,9 +828,11 @@ void policydb_destroy(struct policydb *p)
> hashtab_map(p->range_tr, range_tr_destroy, NULL);
> hashtab_destroy(p->range_tr);
>
> - for (i = 0; i < p->p_types.nprim; i++)
> - ebitmap_destroy(&p->type_attr_map_array[i]);
> - kvfree(p->type_attr_map_array);
> + if (p->type_attr_map_array) {
> + for (i = 0; i < p->p_types.nprim; i++)
> + ebitmap_destroy(&p->type_attr_map_array[i]);
> + kvfree(p->type_attr_map_array);
> + }
>
> ebitmap_destroy(&p->filename_trans_ttypes);
> ebitmap_destroy(&p->policycaps);
> @@ -2496,10 +2498,13 @@ int policydb_read(struct policydb *p, void *fp)
> if (!p->type_attr_map_array)
> goto bad;
>
> + /* just in case ebitmap_init() becomes more than just a memset(0): */
> + for (i = 0; i < p->p_types.nprim; i++)
> + ebitmap_init(&p->type_attr_map_array[i]);
> +
> for (i = 0; i < p->p_types.nprim; i++) {
> struct ebitmap *e = &p->type_attr_map_array[i];
>
> - ebitmap_init(e);
> if (p->policyvers >= POLICYDB_VERSION_AVTAB) {
> rc = ebitmap_read(e, fp);
> if (rc)
>

\
 
 \ /
  Last update: 2019-03-18 14:47    [W:0.048 / U:0.532 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site