Thanks. It is fixed. See attached. ________________________________________ From: William Roberts [[email protected]] Sent: Friday, December 21, 2012 5:46 PM To: Alice Chu Cc: [email protected]; William Roberts Subject: Re: Fixing checkpolicy issues found by Klocwork
Your comments use c99 style instead of c89 style, please change // to /**/ On Fri, Dec 21, 2012 at 5:27 PM, Alice Chu <[email protected]> wrote: > Hello, > > Attached you will find the Klocwork report on external/checkpolicy and my fix > in a patch. The change is done on master branch. > Please review and let me know any additional correction I should make. > > This is how I tested the fix: > (1) Prior to any change, did a full build and saved the following files > aside: > out/target/product/generic/root/file_contexts > out/target/product/generic/root/property_contexts > out/target/product/generic/root/seapp_contexts > out/target/product/generic/root/sepolicy > > out/target/product/generic/obj/ETC/sepolicy_intermediates/policy.conf > (2) Did a clean full build after the change and compared the above files > with the newly created ones. > The test result: the corresponding files are identical. > > Thank you very much for your feedback. > > Regards, > Alice Chu -- Respectfully, William C Roberts
From 5d35bf3dfa88f4494670a2c8db2036c667066488 Mon Sep 17 00:00:00 2001 From: Alice Chu <[email protected]> Date: Fri, 21 Dec 2012 17:59:38 -0800 Subject: [PATCH] Fix issues found by Klocwork Change-Id: Ie31816d34e2470814fba10fb140d181ad5add655 --- module_compiler.c | 6 +++--- policy_define.c | 34 ++++++++++++++++++++++------------ 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/module_compiler.c b/module_compiler.c index ffffaf1..bd758f6 100644 --- a/module_compiler.c +++ b/module_compiler.c @@ -770,19 +770,16 @@ int require_class(int pass) case -3:{ yyerror("Out of memory!"); free(class_id); - class_datum_destroy(datum); goto cleanup; } case -2:{ yyerror("duplicate declaration of class"); free(class_id); - class_datum_destroy(datum); goto cleanup; } case -1:{ yyerror("could not require class here"); free(class_id); - class_datum_destroy(datum); goto cleanup; } case 0:{ @@ -859,6 +856,9 @@ int require_class(int pass) } return 0; cleanup: + if (datum != NULL) { + class_datum_destroy(datum); + } return -1; } diff --git a/policy_define.c b/policy_define.c index 2c12447..4737c59 100644 --- a/policy_define.c +++ b/policy_define.c @@ -1482,7 +1482,7 @@ int define_compute_type_helper(int which, avrule_t ** rule) type_datum_t *datum; ebitmap_t tclasses; ebitmap_node_t *node; - avrule_t *avrule; + avrule_t *avrule = NULL; class_perm_node_t *perm; int i, add = 1; @@ -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; @@ -2035,7 +2035,7 @@ int define_roleattribute(void) role_datum_t *merge_roles_dom(role_datum_t * r1, role_datum_t * r2) { - role_datum_t *new; + role_datum_t *new = NULL; if (pass == 1) { return (role_datum_t *) 1; /* any non-NULL value */ @@ -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; } } @@ -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); @@ -3767,7 +3780,6 @@ int define_fs_context(unsigned int major, unsigned int minor) if (pass == 1) { parse_security_context(NULL); - parse_security_context(NULL); return 0; } @@ -4155,7 +4167,6 @@ int define_netif_context(void) if (pass == 1) { free(queue_remove(id_queue)); parse_security_context(NULL); - parse_security_context(NULL); return 0; } @@ -4215,7 +4226,6 @@ int define_ipv4_node_context() if (pass == 1) { free(queue_remove(id_queue)); - free(queue_remove(id_queue)); parse_security_context(NULL); goto out; } @@ -4301,7 +4311,6 @@ int define_ipv6_node_context(void) if (pass == 1) { free(queue_remove(id_queue)); - free(queue_remove(id_queue)); parse_security_context(NULL); goto out; } @@ -4568,7 +4577,7 @@ int define_range_trans(int class_specified) char *id; level_datum_t *levdatum = 0; class_datum_t *cladatum; - range_trans_rule_t *rule; + range_trans_rule_t *rule = NULL; int l, add = 1; if (!mlspol) { @@ -4673,6 +4682,7 @@ int define_range_trans(int class_specified) out: range_trans_rule_destroy(rule); + free(rule); return -1; } -- 1.7.0.4
