diff mbox

libsemanage: properly check return value of iterate function

Message ID 20171127100102.4409-1-jzarsky@redhat.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Jan Zarsky Nov. 27, 2017, 10:01 a.m. UTC
Function dbase_llist_iterate() iterates over records and checks return
value of iterate function. According to a manpage semanage_iterate(3),
handler can return value 1 for early exit. dbase_llist_iterate()
currently checks for return value > 1, which does not include
expected value 1.

Affected functions:
semanage_bool_iterate_local
semanage_fcontext_iterate
semanage_fcontext_iterate_local
semanage_ibendport_iterate_local
semanage_ibpkey_iterate_local
semanage_iface_iterate_local
semanage_node_iterate_local
semanage_port_iterate_local
semanage_seuser_iterate
semanage_seuser_iterate_local
semanage_user_iterate
semanage_user_iterate_local

Signed-off-by: Jan Zarsky <jzarsky@redhat.com>
---
 libsemanage/src/database_llist.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

William Roberts Nov. 27, 2017, 3:50 p.m. UTC | #1
On Mon, Nov 27, 2017 at 2:01 AM, Jan Zarsky <jzarsky@redhat.com> wrote:
> Function dbase_llist_iterate() iterates over records and checks return
> value of iterate function. According to a manpage semanage_iterate(3),
> handler can return value 1 for early exit. dbase_llist_iterate()
> currently checks for return value > 1, which does not include
> expected value 1.
>
> Affected functions:
> semanage_bool_iterate_local
> semanage_fcontext_iterate
> semanage_fcontext_iterate_local
> semanage_ibendport_iterate_local
> semanage_ibpkey_iterate_local
> semanage_iface_iterate_local
> semanage_node_iterate_local
> semanage_port_iterate_local
> semanage_seuser_iterate
> semanage_seuser_iterate_local
> semanage_user_iterate
> semanage_user_iterate_local

Not really what I had in mind. I meant what was the affect. This is simple
enough to gather, so ack on v1,

My understanding is that the affect is that it that it doesn't short
circuit the iterate
routine so lockups take longer than they need be, is that correct?

>
> Signed-off-by: Jan Zarsky <jzarsky@redhat.com>
> ---
>  libsemanage/src/database_llist.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libsemanage/src/database_llist.c b/libsemanage/src/database_llist.c
> index 8ce2e2c1..c8f4ff0b 100644
> --- a/libsemanage/src/database_llist.c
> +++ b/libsemanage/src/database_llist.c
> @@ -263,7 +263,7 @@ int dbase_llist_iterate(semanage_handle_t * handle,
>                 if (rc < 0)
>                         goto err;
>
> -               else if (rc > 1)
> +               else if (rc > 0)
>                         break;
>         }
>
> --
> 2.14.3
>
>
Jan Zarsky Dec. 11, 2017, 1:47 p.m. UTC | #2
----- Original Message -----
> On Mon, Nov 27, 2017 at 2:01 AM, Jan Zarsky <jzarsky@redhat.com> wrote:
> > Function dbase_llist_iterate() iterates over records and checks return
> > value of iterate function. According to a manpage semanage_iterate(3),
> > handler can return value 1 for early exit. dbase_llist_iterate()
> > currently checks for return value > 1, which does not include
> > expected value 1.
> >
> > Affected functions:
> > semanage_bool_iterate_local
> > semanage_fcontext_iterate
> > semanage_fcontext_iterate_local
> > semanage_ibendport_iterate_local
> > semanage_ibpkey_iterate_local
> > semanage_iface_iterate_local
> > semanage_node_iterate_local
> > semanage_port_iterate_local
> > semanage_seuser_iterate
> > semanage_seuser_iterate_local
> > semanage_user_iterate
> > semanage_user_iterate_local
> 
> Not really what I had in mind. I meant what was the affect. This is simple
> enough to gather, so ack on v1,
> 
> My understanding is that the affect is that it that it doesn't short
> circuit the iterate
> routine so lockups take longer than they need be, is that correct?

Yes, that is the exactly the problem. I will try to be more clear next time.

> 
> >
> > Signed-off-by: Jan Zarsky <jzarsky@redhat.com>
> > ---
> >  libsemanage/src/database_llist.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libsemanage/src/database_llist.c
> > b/libsemanage/src/database_llist.c
> > index 8ce2e2c1..c8f4ff0b 100644
> > --- a/libsemanage/src/database_llist.c
> > +++ b/libsemanage/src/database_llist.c
> > @@ -263,7 +263,7 @@ int dbase_llist_iterate(semanage_handle_t * handle,
> >                 if (rc < 0)
> >                         goto err;
> >
> > -               else if (rc > 1)
> > +               else if (rc > 0)
> >                         break;
> >         }
> >
> > --
> > 2.14.3
> >
> >
> 
> 
> 
> --
> Respectfully,
> 
> William C Roberts
>
diff mbox

Patch

diff --git a/libsemanage/src/database_llist.c b/libsemanage/src/database_llist.c
index 8ce2e2c1..c8f4ff0b 100644
--- a/libsemanage/src/database_llist.c
+++ b/libsemanage/src/database_llist.c
@@ -263,7 +263,7 @@  int dbase_llist_iterate(semanage_handle_t * handle,
 		if (rc < 0)
 			goto err;
 
-		else if (rc > 1)
+		else if (rc > 0)
 			break;
 	}