Return failure in getnetgrent only when all netgroups have been searched (#17363)
diff mbox

Message ID 20140908171437.GD9978@spoyarek.pnq.redhat.com
State New
Headers show

Commit Message

Siddhesh Poyarekar Sept. 8, 2014, 5:14 p.m. UTC
Hi,

The netgroups lookup code fails when one of the groups in the search
tree is empty.  In such a case it only returns the leaves of the tree
after the blank netgroup.  This is because the line parser returns a
NOTFOUND status when the netgroup exists but is empty.  The
__getnetgrent_internal implementation needs to be fixed to try
remaining groups if the current group is entry.  This patch implements
this fix.  Tested on x86_64.

Siddhesh

	[BZ #17363]
	* inet/getnetgrent_r.c (__internal_getnetgrent_r): Try next
	group if the current group is empty.

Comments

Carlos O'Donell Sept. 10, 2014, 4:09 p.m. UTC | #1
On 09/08/2014 01:14 PM, Siddhesh Poyarekar wrote:
> Hi,
> 
> The netgroups lookup code fails when one of the groups in the search
> tree is empty.  In such a case it only returns the leaves of the tree
> after the blank netgroup.  This is because the line parser returns a
> NOTFOUND status when the netgroup exists but is empty.  The
> __getnetgrent_internal implementation needs to be fixed to try
> remaining groups if the current group is entry.  This patch implements
> this fix.  Tested on x86_64.
> 
> Siddhesh
> 
> 	[BZ #17363]
> 	* inet/getnetgrent_r.c (__internal_getnetgrent_r): Try next
> 	group if the current group is empty.
> 
> diff --git a/inet/getnetgrent_r.c b/inet/getnetgrent_r.c
> index f6d064d..e101537 100644
> --- a/inet/getnetgrent_r.c
> +++ b/inet/getnetgrent_r.c
> @@ -297,7 +297,10 @@ __internal_getnetgrent_r (char **hostp, char **userp, char **domainp,
>      {
>        status = DL_CALL_FCT (*fct, (datap, buffer, buflen, &errno));
>  
> -      if (status == NSS_STATUS_RETURN)
> +      if (status == NSS_STATUS_RETURN
> +	  /* The service returned a NOTFOUND, but there are more groups that we
> +	     need to resolve before we give up.  */

OK but suggest expanded comment:

/* An empty group returns NOTFOUND, but there may be more groups that we
   need to resolve before we can return NOTFOUND.  */

> +	  || (status == NSS_STATUS_NOTFOUND && datap->needed_groups != NULL))
>  	{
>  	  /* This was the last one for this group.  Look at next group
>  	     if available.  */
> 

OK.

Returning NSS_STATUS_NOTFOUND for an empty group seems reasonable from the
internals interface perspective, and the code in __internal_getnetgrent_r
doesn't handle it properly.

Even if NSS_STATUS_NOTFOUND is returned, we should keep parsing because
there might be other groups to consider.

Looks good to me.

Cheers,
Carlos.

Patch
diff mbox

diff --git a/inet/getnetgrent_r.c b/inet/getnetgrent_r.c
index f6d064d..e101537 100644
--- a/inet/getnetgrent_r.c
+++ b/inet/getnetgrent_r.c
@@ -297,7 +297,10 @@  __internal_getnetgrent_r (char **hostp, char **userp, char **domainp,
     {
       status = DL_CALL_FCT (*fct, (datap, buffer, buflen, &errno));
 
-      if (status == NSS_STATUS_RETURN)
+      if (status == NSS_STATUS_RETURN
+	  /* The service returned a NOTFOUND, but there are more groups that we
+	     need to resolve before we give up.  */
+	  || (status == NSS_STATUS_NOTFOUND && datap->needed_groups != NULL))
 	{
 	  /* This was the last one for this group.  Look at next group
 	     if available.  */