Hi Stephen, Thanks for pointing out the error. Here is the change (with a more specific commit comment).
>From 0d73ef7049feee794f14cf1af88d05dae8139914 Mon Sep 17 00:00:00 2001 From: Alice Chu <[email protected]> Date: Tue, 8 Jan 2013 16:05:25 -0800 Subject: [PATCH] Free allocated memory when clean up / exit. Change-Id: I116a3e1fa17f357b10e7414efde6c78ffb9678df --- policy_define.c | 25 ++++++++++++++++++++----- 1 files changed, 20 insertions(+), 5 deletions(-) diff --git a/policy_define.c b/policy_define.c index 2c12447..8af6141 100644 --- a/policy_define.c +++ b/policy_define.c @@ -1497,12 +1497,12 @@ int define_compute_type_helper(int which, avrule_t ** rule) while ((id = queue_remove(id_queue))) { if (set_types(&avrule->stypes, id, &add, 0)) - return -1; + goto bad; } add = 1; while ((id = queue_remove(id_queue))) { if (set_types(&avrule->ttypes, id, &add, 0)) - return -1; + goto bad; } ebitmap_init(&tclasses); @@ -1531,7 +1531,7 @@ int define_compute_type_helper(int which, avrule_t ** rule) perm = malloc(sizeof(class_perm_node_t)); if (!perm) { yyerror("out of memory"); - return -1; + goto bad; } class_perm_node_init(perm); perm->class = i + 1; @@ -2050,10 +2050,12 @@ role_datum_t *merge_roles_dom(role_datum_t * r1, role_datum_t * r2) new->s.value = 0; /* temporary role */ if (ebitmap_or(&new->dominates, &r1->dominates, &r2->dominates)) { yyerror("out of memory"); + free(new); return NULL; } if (ebitmap_or(&new->types.types, &r1->types.types, &r2->types.types)) { yyerror("out of memory"); + free(new); return NULL; } if (!r1->s.value) { @@ -2458,13 +2460,17 @@ int define_role_allow(void) role_allow_rule_init(ra); while ((id = queue_remove(id_queue))) { - if (set_roles(&ra->roles, id)) + if (set_roles(&ra->roles, id)) { + free(ra); return -1; + } } while ((id = queue_remove(id_queue))) { - if (set_roles(&ra->new_roles, id)) + if (set_roles(&ra->new_roles, id)) { + free(ra); return -1; + } } append_role_allow(ra); @@ -2777,6 +2783,7 @@ int define_constraint(constraint_expr_t * expr) } if (!node->expr) { yyerror("out of memory"); + free(node); return -1; } node->permissions = 0; @@ -3281,6 +3288,7 @@ cond_expr_t *define_cond_expr(uint32_t expr_type, void *arg1, void *arg2) return expr; default: yyerror("illegal conditional expression"); + free(expr); return NULL; } } @@ -3582,6 +3590,12 @@ static int parse_security_context(context_struct_t * c) return 0; } + /* check context c to make sure ok to dereference c later */ + if (c == NULL) { + yyerror("null context pointer!"); + return -1; + } + context_init(c); /* extract the user */ @@ -4673,6 +4687,7 @@ int define_range_trans(int class_specified) out: range_trans_rule_destroy(rule); + free(rule); return -1; } -- 1.7.0.4 Thanks, Alice ________________________________________ From: Stephen Smalley [[email protected]] Sent: Tuesday, January 08, 2013 7:07 AM To: Alice Chu Cc: [email protected]; [email protected] Subject: Re: Fixing external/checkpolicy issues found by Klocwork On 01/07/2013 08:29 PM, Alice Chu wrote: > Hello, > > Attached you will find the Klocwork report on seandroid master branch > external/checkpolicy. The following is the fix for issues found in > policy_define.c. > Please review and give me your feedback. > > Thank you very much, > Alice Chu > > ============================================================================ >>From 18555451c5831fd95044e665d3dc514eb69e3b75 Mon Sep 17 00:00:00 2001 > From: Alice Chu <[email protected]> > Date: Mon, 7 Jan 2013 15:29:29 -0800 > Subject: [PATCH] Fix issues found by Klocwork > > Change-Id: Ic3a01364b6855529f6b58a8820c6011a22c21841 > --- > policy_define.c | 24 +++++++++++++++++++----- > 1 files changed, 19 insertions(+), 5 deletions(-) > > diff --git a/policy_define.c b/policy_define.c > index 2c12447..504af69 100644 > --- a/policy_define.c > +++ b/policy_define.c > @@ -3583,6 +3591,11 @@ static int parse_security_context(context_struct_t * c) > } > > context_init(c); > + /* check context c to make sure ok to dereference c later */ > + if (c == NULL) { > + yyerror("null context pointer!"); > + goto bad; > + } > > /* extract the user */ > id = queue_remove(id_queue); I think you want this check before context_init(), as it dereferences c. And then just return -1 in the error path. This btw is an illegal state as NULL should only be passed if pass == 1. -- This message was distributed to subscribers of the seandroid-list mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe seandroid-list" without quotes as the message.
