All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] libsemanage: properly check return value of iterate function
@ 2017-11-22 15:09 Jan Zarsky
  2017-11-22 21:35 ` William Roberts
  2017-11-27 19:09 ` William Roberts
  0 siblings, 2 replies; 6+ messages in thread
From: Jan Zarsky @ 2017-11-22 15:09 UTC (permalink / raw)
  To: selinux

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. This affects most of the semanage_*_iterate
and semanage_*_local functions.

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] libsemanage: properly check return value of iterate function
  2017-11-22 15:09 [PATCH] libsemanage: properly check return value of iterate function Jan Zarsky
@ 2017-11-22 21:35 ` William Roberts
  2017-11-27 10:01   ` Jan Zarsky
  2017-11-27 19:09 ` William Roberts
  1 sibling, 1 reply; 6+ messages in thread
From: William Roberts @ 2017-11-22 21:35 UTC (permalink / raw)
  To: Jan Zarsky; +Cc: selinux@tycho.nsa.gov

On Wed, Nov 22, 2017 at 7:09 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. This affects most of the semanage_*_iterate
> and semanage_*_local functions.

Can you update this message to describe what is affected.

>
> 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)

This looks fine to me.

>                         break;
>         }
>
> --
> 2.14.3
>

Please resend with the message updated and I'll ack.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] libsemanage: properly check return value of iterate function
  2017-11-22 21:35 ` William Roberts
@ 2017-11-27 10:01   ` Jan Zarsky
  2017-11-27 15:50     ` William Roberts
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Zarsky @ 2017-11-27 10:01 UTC (permalink / raw)
  To: selinux

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(-)

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] libsemanage: properly check return value of iterate function
  2017-11-27 10:01   ` Jan Zarsky
@ 2017-11-27 15:50     ` William Roberts
  2017-12-11 13:47       ` Jan Zarsky
  0 siblings, 1 reply; 6+ messages in thread
From: William Roberts @ 2017-11-27 15:50 UTC (permalink / raw)
  To: Jan Zarsky; +Cc: selinux@tycho.nsa.gov

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
>
>



-- 
Respectfully,

William C Roberts

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] libsemanage: properly check return value of iterate function
  2017-11-22 15:09 [PATCH] libsemanage: properly check return value of iterate function Jan Zarsky
  2017-11-22 21:35 ` William Roberts
@ 2017-11-27 19:09 ` William Roberts
  1 sibling, 0 replies; 6+ messages in thread
From: William Roberts @ 2017-11-27 19:09 UTC (permalink / raw)
  To: Jan Zarsky; +Cc: selinux@tycho.nsa.gov

Thanks. Applied: https://github.com/SELinuxProject/selinux/pull/71

On Wed, Nov 22, 2017 at 7:09 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. This affects most of the semanage_*_iterate
> and semanage_*_local functions.
>
> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] libsemanage: properly check return value of iterate function
  2017-11-27 15:50     ` William Roberts
@ 2017-12-11 13:47       ` Jan Zarsky
  0 siblings, 0 replies; 6+ messages in thread
From: Jan Zarsky @ 2017-12-11 13:47 UTC (permalink / raw)
  To: William Roberts; +Cc: selinux

----- 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
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-12-11 13:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-22 15:09 [PATCH] libsemanage: properly check return value of iterate function Jan Zarsky
2017-11-22 21:35 ` William Roberts
2017-11-27 10:01   ` Jan Zarsky
2017-11-27 15:50     ` William Roberts
2017-12-11 13:47       ` Jan Zarsky
2017-11-27 19:09 ` William Roberts

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.