* [PATCH v2 1/2] libsepol/cil: Initialize the multiple_decls field of the cil db
@ 2020-05-19 14:29 James Carter
2020-05-19 14:29 ` [PATCH v2 2/2] libsepol/cil: Return error when identifier declared as both type and attribute James Carter
2020-05-26 15:41 ` [PATCH v2 1/2] libsepol/cil: Initialize the multiple_decls field of the cil db Stephen Smalley
0 siblings, 2 replies; 5+ messages in thread
From: James Carter @ 2020-05-19 14:29 UTC (permalink / raw)
To: selinux; +Cc: James Carter
Initialize the multiple_decls field when intializing the structure
cil_db.
Issue reported by: Topi Miettinen <toiwoton@gmail.com>
Fixes: fafe4c212bf6c32c ("libsepol: cil: Add ability to redeclare
types[attributes]")
Signed-off-by: James Carter <jwcart2@gmail.com>
---
libsepol/cil/src/cil.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/libsepol/cil/src/cil.c b/libsepol/cil/src/cil.c
index 4a77aa9c..a3c6a293 100644
--- a/libsepol/cil/src/cil.c
+++ b/libsepol/cil/src/cil.c
@@ -459,6 +459,7 @@ void cil_db_init(struct cil_db **db)
(*db)->preserve_tunables = CIL_FALSE;
(*db)->handle_unknown = -1;
(*db)->mls = -1;
+ (*db)->multiple_decls = CIL_FALSE;
(*db)->target_platform = SEPOL_TARGET_SELINUX;
(*db)->policy_version = POLICYDB_VERSION_MAX;
}
--
2.25.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] libsepol/cil: Return error when identifier declared as both type and attribute
2020-05-19 14:29 [PATCH v2 1/2] libsepol/cil: Initialize the multiple_decls field of the cil db James Carter
@ 2020-05-19 14:29 ` James Carter
2020-05-26 15:47 ` Stephen Smalley
2020-05-26 15:41 ` [PATCH v2 1/2] libsepol/cil: Initialize the multiple_decls field of the cil db Stephen Smalley
1 sibling, 1 reply; 5+ messages in thread
From: James Carter @ 2020-05-19 14:29 UTC (permalink / raw)
To: selinux; +Cc: James Carter
CIL allows a type to be redeclared when using the multiple declarations
option ("-m" or "--muliple-decls"), but make it an error for an identifier
to be declared as both a type and an attribute.
Change the error message so that it always gives the location and flavor
of both declarations. The flavors will be the same in all other cases,
but in this case they explain why there is an error even if multiple
declartions are allowed.
Issue reported by: Topi Miettinen <toiwoton@gmail.com>
Fixes: Commit fafe4c212bf6c32c ("libsepol: cil: Add ability to redeclare
types[attributes]")
Signed-off-by: James Carter <jwcart2@gmail.com>
---
libsepol/cil/src/cil_build_ast.c | 29 +++++++++++++++++------------
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
index fcecdc4f..ce2499a1 100644
--- a/libsepol/cil/src/cil_build_ast.c
+++ b/libsepol/cil/src/cil_build_ast.c
@@ -87,7 +87,7 @@ exit:
* datum, given the new datum and the one already present in a given symtab.
*/
int cil_is_datum_multiple_decl(__attribute__((unused)) struct cil_symtab_datum *cur,
- __attribute__((unused)) struct cil_symtab_datum *old,
+ struct cil_symtab_datum *old,
enum cil_flavor f)
{
int rc = CIL_FALSE;
@@ -95,8 +95,12 @@ int cil_is_datum_multiple_decl(__attribute__((unused)) struct cil_symtab_datum *
switch (f) {
case CIL_TYPE:
case CIL_TYPEATTRIBUTE:
- /* type and typeattribute statements insert empty datums, ret true */
- rc = CIL_TRUE;
+ if (!old || f != FLAVOR(old)) {
+ rc = CIL_FALSE;
+ } else {
+ /* type and typeattribute statements insert empty datums */
+ rc = CIL_TRUE;
+ }
break;
default:
break;
@@ -126,19 +130,20 @@ int cil_gen_node(struct cil_db *db, struct cil_tree_node *ast_node, struct cil_s
if (symtab != NULL) {
rc = cil_symtab_insert(symtab, (hashtab_key_t)key, datum, ast_node);
if (rc == SEPOL_EEXIST) {
+ rc = cil_symtab_get_datum(symtab, (hashtab_key_t)key, &prev);
+ if (rc != SEPOL_OK) {
+ cil_log(CIL_ERR, "Re-declaration of %s %s, but previous declaration could not be found\n",cil_node_to_string(ast_node), key);
+ goto exit;
+ }
if (!db->multiple_decls ||
- cil_symtab_get_datum(symtab, (hashtab_key_t)key, &prev) != SEPOL_OK ||
!cil_is_datum_multiple_decl(datum, prev, nflavor)) {
-
/* multiple_decls not ok, ret error */
+ struct cil_tree_node *node = NODE(prev);
cil_log(CIL_ERR, "Re-declaration of %s %s\n",
cil_node_to_string(ast_node), key);
- if (cil_symtab_get_datum(symtab, key, &datum) == SEPOL_OK) {
- if (sflavor == CIL_SYM_BLOCKS) {
- struct cil_tree_node *node = datum->nodes->head->data;
- cil_tree_log(node, CIL_ERR, "Previous declaration");
- }
- }
+ cil_tree_log(node, CIL_ERR, "Previous declaration of %s",
+ cil_node_to_string(node));
+ rc = SEPOL_ERR;
goto exit;
}
/* multiple_decls is enabled and works for this datum type, add node */
@@ -169,7 +174,7 @@ int cil_gen_node(struct cil_db *db, struct cil_tree_node *ast_node, struct cil_s
return SEPOL_OK;
exit:
- cil_log(CIL_ERR, "Failed to create node\n");
+ cil_log(CIL_INFO, "Failed to create node\n");
return rc;
}
--
2.25.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] libsepol/cil: Initialize the multiple_decls field of the cil db
2020-05-19 14:29 [PATCH v2 1/2] libsepol/cil: Initialize the multiple_decls field of the cil db James Carter
2020-05-19 14:29 ` [PATCH v2 2/2] libsepol/cil: Return error when identifier declared as both type and attribute James Carter
@ 2020-05-26 15:41 ` Stephen Smalley
1 sibling, 0 replies; 5+ messages in thread
From: Stephen Smalley @ 2020-05-26 15:41 UTC (permalink / raw)
To: James Carter; +Cc: SElinux list
On Tue, May 19, 2020 at 10:30 AM James Carter <jwcart2@gmail.com> wrote:
>
> Initialize the multiple_decls field when intializing the structure
> cil_db.
>
> Issue reported by: Topi Miettinen <toiwoton@gmail.com>
Conventionally this is just Reported-by: ...
Otherwise,
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> Fixes: fafe4c212bf6c32c ("libsepol: cil: Add ability to redeclare
> types[attributes]")
>
> Signed-off-by: James Carter <jwcart2@gmail.com>
> ---
> libsepol/cil/src/cil.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/libsepol/cil/src/cil.c b/libsepol/cil/src/cil.c
> index 4a77aa9c..a3c6a293 100644
> --- a/libsepol/cil/src/cil.c
> +++ b/libsepol/cil/src/cil.c
> @@ -459,6 +459,7 @@ void cil_db_init(struct cil_db **db)
> (*db)->preserve_tunables = CIL_FALSE;
> (*db)->handle_unknown = -1;
> (*db)->mls = -1;
> + (*db)->multiple_decls = CIL_FALSE;
> (*db)->target_platform = SEPOL_TARGET_SELINUX;
> (*db)->policy_version = POLICYDB_VERSION_MAX;
> }
> --
> 2.25.4
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] libsepol/cil: Return error when identifier declared as both type and attribute
2020-05-19 14:29 ` [PATCH v2 2/2] libsepol/cil: Return error when identifier declared as both type and attribute James Carter
@ 2020-05-26 15:47 ` Stephen Smalley
2020-05-26 17:21 ` James Carter
0 siblings, 1 reply; 5+ messages in thread
From: Stephen Smalley @ 2020-05-26 15:47 UTC (permalink / raw)
To: James Carter; +Cc: SElinux list
On Tue, May 19, 2020 at 10:30 AM James Carter <jwcart2@gmail.com> wrote:
>
> CIL allows a type to be redeclared when using the multiple declarations
> option ("-m" or "--muliple-decls"), but make it an error for an identifier
> to be declared as both a type and an attribute.
>
> Change the error message so that it always gives the location and flavor
> of both declarations. The flavors will be the same in all other cases,
> but in this case they explain why there is an error even if multiple
> declartions are allowed.
>
> Issue reported by: Topi Miettinen <toiwoton@gmail.com>
Normally just Reported-by:...
> Fixes: Commit fafe4c212bf6c32c ("libsepol: cil: Add ability to redeclare
> types[attributes]")
Normally just "Fixes: <hash> ("one-liner")", no "Commit".
> Signed-off-by: James Carter <jwcart2@gmail.com>
> ---
> libsepol/cil/src/cil_build_ast.c | 29 +++++++++++++++++------------
> 1 file changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
> index fcecdc4f..ce2499a1 100644
> --- a/libsepol/cil/src/cil_build_ast.c
> +++ b/libsepol/cil/src/cil_build_ast.c
> @@ -87,7 +87,7 @@ exit:
> * datum, given the new datum and the one already present in a given symtab.
> */
> int cil_is_datum_multiple_decl(__attribute__((unused)) struct cil_symtab_datum *cur,
> - __attribute__((unused)) struct cil_symtab_datum *old,
> + struct cil_symtab_datum *old,
> enum cil_flavor f)
> {
> int rc = CIL_FALSE;
> @@ -95,8 +95,12 @@ int cil_is_datum_multiple_decl(__attribute__((unused)) struct cil_symtab_datum *
> switch (f) {
> case CIL_TYPE:
> case CIL_TYPEATTRIBUTE:
> - /* type and typeattribute statements insert empty datums, ret true */
> - rc = CIL_TRUE;
> + if (!old || f != FLAVOR(old)) {
> + rc = CIL_FALSE;
> + } else {
> + /* type and typeattribute statements insert empty datums */
> + rc = CIL_TRUE;
> + }
> break;
> default:
> break;
> @@ -126,19 +130,20 @@ int cil_gen_node(struct cil_db *db, struct cil_tree_node *ast_node, struct cil_s
> if (symtab != NULL) {
> rc = cil_symtab_insert(symtab, (hashtab_key_t)key, datum, ast_node);
> if (rc == SEPOL_EEXIST) {
> + rc = cil_symtab_get_datum(symtab, (hashtab_key_t)key, &prev);
> + if (rc != SEPOL_OK) {
> + cil_log(CIL_ERR, "Re-declaration of %s %s, but previous declaration could not be found\n",cil_node_to_string(ast_node), key);
> + goto exit;
> + }
> if (!db->multiple_decls ||
> - cil_symtab_get_datum(symtab, (hashtab_key_t)key, &prev) != SEPOL_OK ||
> !cil_is_datum_multiple_decl(datum, prev, nflavor)) {
> -
> /* multiple_decls not ok, ret error */
> + struct cil_tree_node *node = NODE(prev);
> cil_log(CIL_ERR, "Re-declaration of %s %s\n",
> cil_node_to_string(ast_node), key);
> - if (cil_symtab_get_datum(symtab, key, &datum) == SEPOL_OK) {
> - if (sflavor == CIL_SYM_BLOCKS) {
> - struct cil_tree_node *node = datum->nodes->head->data;
> - cil_tree_log(node, CIL_ERR, "Previous declaration");
> - }
> - }
> + cil_tree_log(node, CIL_ERR, "Previous declaration of %s",
> + cil_node_to_string(node));
> + rc = SEPOL_ERR;
> goto exit;
> }
> /* multiple_decls is enabled and works for this datum type, add node */
> @@ -169,7 +174,7 @@ int cil_gen_node(struct cil_db *db, struct cil_tree_node *ast_node, struct cil_s
> return SEPOL_OK;
>
> exit:
> - cil_log(CIL_ERR, "Failed to create node\n");
> + cil_log(CIL_INFO, "Failed to create node\n");
Is this useful/meaningful to retain? Seems odd to have an
informational message about a failure to create a node.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] libsepol/cil: Return error when identifier declared as both type and attribute
2020-05-26 15:47 ` Stephen Smalley
@ 2020-05-26 17:21 ` James Carter
0 siblings, 0 replies; 5+ messages in thread
From: James Carter @ 2020-05-26 17:21 UTC (permalink / raw)
To: Stephen Smalley; +Cc: SElinux list
On Tue, May 26, 2020 at 11:47 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Tue, May 19, 2020 at 10:30 AM James Carter <jwcart2@gmail.com> wrote:
> >
> > CIL allows a type to be redeclared when using the multiple declarations
> > option ("-m" or "--muliple-decls"), but make it an error for an identifier
> > to be declared as both a type and an attribute.
> >
> > Change the error message so that it always gives the location and flavor
> > of both declarations. The flavors will be the same in all other cases,
> > but in this case they explain why there is an error even if multiple
> > declartions are allowed.
> >
> > Issue reported by: Topi Miettinen <toiwoton@gmail.com>
>
> Normally just Reported-by:...
>
> > Fixes: Commit fafe4c212bf6c32c ("libsepol: cil: Add ability to redeclare
> > types[attributes]")
>
> Normally just "Fixes: <hash> ("one-liner")", no "Commit".
>
> > Signed-off-by: James Carter <jwcart2@gmail.com>
> > ---
> > libsepol/cil/src/cil_build_ast.c | 29 +++++++++++++++++------------
> > 1 file changed, 17 insertions(+), 12 deletions(-)
> >
> > diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
> > index fcecdc4f..ce2499a1 100644
> > --- a/libsepol/cil/src/cil_build_ast.c
> > +++ b/libsepol/cil/src/cil_build_ast.c
> > @@ -87,7 +87,7 @@ exit:
> > * datum, given the new datum and the one already present in a given symtab.
> > */
> > int cil_is_datum_multiple_decl(__attribute__((unused)) struct cil_symtab_datum *cur,
> > - __attribute__((unused)) struct cil_symtab_datum *old,
> > + struct cil_symtab_datum *old,
> > enum cil_flavor f)
> > {
> > int rc = CIL_FALSE;
> > @@ -95,8 +95,12 @@ int cil_is_datum_multiple_decl(__attribute__((unused)) struct cil_symtab_datum *
> > switch (f) {
> > case CIL_TYPE:
> > case CIL_TYPEATTRIBUTE:
> > - /* type and typeattribute statements insert empty datums, ret true */
> > - rc = CIL_TRUE;
> > + if (!old || f != FLAVOR(old)) {
> > + rc = CIL_FALSE;
> > + } else {
> > + /* type and typeattribute statements insert empty datums */
> > + rc = CIL_TRUE;
> > + }
> > break;
> > default:
> > break;
> > @@ -126,19 +130,20 @@ int cil_gen_node(struct cil_db *db, struct cil_tree_node *ast_node, struct cil_s
> > if (symtab != NULL) {
> > rc = cil_symtab_insert(symtab, (hashtab_key_t)key, datum, ast_node);
> > if (rc == SEPOL_EEXIST) {
> > + rc = cil_symtab_get_datum(symtab, (hashtab_key_t)key, &prev);
> > + if (rc != SEPOL_OK) {
> > + cil_log(CIL_ERR, "Re-declaration of %s %s, but previous declaration could not be found\n",cil_node_to_string(ast_node), key);
> > + goto exit;
> > + }
> > if (!db->multiple_decls ||
> > - cil_symtab_get_datum(symtab, (hashtab_key_t)key, &prev) != SEPOL_OK ||
> > !cil_is_datum_multiple_decl(datum, prev, nflavor)) {
> > -
> > /* multiple_decls not ok, ret error */
> > + struct cil_tree_node *node = NODE(prev);
> > cil_log(CIL_ERR, "Re-declaration of %s %s\n",
> > cil_node_to_string(ast_node), key);
> > - if (cil_symtab_get_datum(symtab, key, &datum) == SEPOL_OK) {
> > - if (sflavor == CIL_SYM_BLOCKS) {
> > - struct cil_tree_node *node = datum->nodes->head->data;
> > - cil_tree_log(node, CIL_ERR, "Previous declaration");
> > - }
> > - }
> > + cil_tree_log(node, CIL_ERR, "Previous declaration of %s",
> > + cil_node_to_string(node));
> > + rc = SEPOL_ERR;
> > goto exit;
> > }
> > /* multiple_decls is enabled and works for this datum type, add node */
> > @@ -169,7 +174,7 @@ int cil_gen_node(struct cil_db *db, struct cil_tree_node *ast_node, struct cil_s
> > return SEPOL_OK;
> >
> > exit:
> > - cil_log(CIL_ERR, "Failed to create node\n");
> > + cil_log(CIL_INFO, "Failed to create node\n");
>
> Is this useful/meaningful to retain? Seems odd to have an
> informational message about a failure to create a node.
I definitely did not think that there was any value in reporting it as
an error and I agree with you that it is not very useful at all.
Jim
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-05-26 17:21 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-19 14:29 [PATCH v2 1/2] libsepol/cil: Initialize the multiple_decls field of the cil db James Carter
2020-05-19 14:29 ` [PATCH v2 2/2] libsepol/cil: Return error when identifier declared as both type and attribute James Carter
2020-05-26 15:47 ` Stephen Smalley
2020-05-26 17:21 ` James Carter
2020-05-26 15:41 ` [PATCH v2 1/2] libsepol/cil: Initialize the multiple_decls field of the cil db Stephen Smalley
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.