On 01/17/2017 04:47 PM, Stephen Smalley wrote:
On Sat, 2017-01-14 at 14:12 +0100, Nicolas Iooss wrote:This kind of strange construction is currently accepted by checkmodule but it makes memory to be leaked in declare_type(): optional { require { type TYPE1; } } optional { require { attribute ATTR; } type TYPE1, ATTR; } Moreover reversing the order of these two optional blocks leads to an (expected) error: ERROR 'duplicate declaration of type/attribute' at token 'TYPE1' on line 14: require { type TYPE1; }I could be wrong, but this error seems wrong to me. It isn't declaring the type multiple times but instead is requiring it in one optional block (with an empty body besides the require) and then declaring it in another. Technically, that should turn on the first optional block during link/expand IIUC. cc'ing some others
The error looks wrong to me as well. Jim
Make checkmodule fails with an error when encourntering the first construction. Moreover the construction below also makes memory to be leaked: optional { require { type TYPE2; attribute ATTR; } typeattribute TYPE2 ATTR; } type TYPE2; This example is valid and is used by several refpolicy modules. In declare_type(), declare_symbol() returns 1, stack_top->parent is NULL and the scope of TYPE2 has been updated when declare_symbol() called symtab_insert(). When these conditions are met, declare_type() should return the currently existing type in the policy database. Signed-off-by: Nicolas Iooss <nicolas.iooss@xxxxxxx> --- I have tested this patch both with "make test" and by compiling the current master branch of refpolicy. I have not tested it to build any Android policy. checkpolicy/module_compiler.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/checkpolicy/module_compiler.c b/checkpolicy/module_compiler.c index 6e5483c17540..04d09d1445a3 100644 --- a/checkpolicy/module_compiler.c +++ b/checkpolicy/module_compiler.c @@ -308,7 +308,7 @@ role_datum_t *declare_role(unsigned char isattr) type_datum_t *declare_type(unsigned char primary, unsigned char isattr) { char *id; - type_datum_t *typdatum; + type_datum_t *typdatum, *old_datum; int retval; uint32_t value = 0; @@ -335,10 +335,37 @@ type_datum_t *declare_type(unsigned char primary, unsigned char isattr) typdatum->flavor = isattr ? TYPE_ATTRIB : TYPE_TYPE; retval = declare_symbol(SYM_TYPES, id, typdatum, &value, &value); - if (retval == 0 || retval == 1) { + if (retval == 0) { if (typdatum->primary) { typdatum->s.value = value; } + } else if (retval == 1) { + /* the type has been previously marked as required */ + assert(stack_top->type == 1); + if (stack_top->parent == NULL) { + /* in global scope, the type has previously been required and is now declared. + * policydbp->scope[SYM_TYPES] has been updated by declare_symbol() */ + old_datum = (type_datum_t *) hashtab_search(policydbp->p_types.table, id); + + /* check that previous symbol has the same type/attribute-ness */ + if (typdatum->flavor == old_datum->flavor) { + type_datum_destroy(typdatum); + free(typdatum); + typdatum = old_datum; + } else { + yyerror("declaration of type/attribute with an unexpected flavor"); + type_datum_destroy(typdatum); + free(typdatum); + typdatum = NULL; + } + free(id); + } else { + yyerror("declaration of type/attribute after it has been required"); + free(id); + type_datum_destroy(typdatum); + free(typdatum); + return NULL; + } } else { /* error occurred (can't have duplicate type declarations) */ free(id);
-- James Carter <jwcart2@xxxxxxxxxxxxx> National Security Agency _______________________________________________ Selinux mailing list Selinux@xxxxxxxxxxxxx To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx. To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.
