Bug 26486 - [libc] [patch] setnetgrent hangs when netgroup contains empty netgroup
Summary: [libc] [patch] setnetgrent hangs when netgroup contains empty netgroup
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 4.2-RELEASE
Hardware: Any Any
: Normal Affects Only Me
Assignee: Mark Johnston
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2001-04-10 17:50 UTC by gil
Modified: 2016-08-02 21:18 UTC (History)
4 users (show)

See Also:


Attachments
Updated patch to solve hang on empty netgroups for FreeBSD 10 (1.38 KB, patch)
2015-02-19 21:39 UTC, gil
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description gil 2001-04-10 17:50:01 UTC
	NIS (yp) netgroups set-up as follows:

	ssh-users	adg-ssh-users omg-ssh-users
	adg-ssh-users
	omg-ssh-users	(-,foo,) (-,bar,)

	A call to setnetgrent("ssh-users") will hang due to the empty
	adg-ssh-users netgroup.  However, it does work when accessed
	via /etc/passwd using the +@ syntax...

Fix: 

Currently investigating where the hang is occuring.  Will update
	as soon as I find out more, unless someone else finds it first
	and lets me know.  Probably in the parse_netgrp function in
	/usr/src/lib/libc/gen/getnetgrent.c
How-To-Repeat: 
	using above conditions...

	#include <stdlib.h>
	#include <stdio.h>

	main()
	{
        	char            *nh, *nu, *nd;

        	printf("1\n");
        	setnetgrent("ssh-users");

        	printf("2\n");
        	nh = nu = nd = (char *)0;
        	while (getnetgrent(&nh, &nu, &nd) > 0) {
                	printf("3\n");
                	printf("> %s\n", nu);
        	}

        	printf("4\n");
        	endnetgrent();

        	printf("5\n");
        	exit(0);
	}
Comment 1 gil 2001-04-11 00:31:59 UTC
The problem appears to be corrected in the unified diff below.  The
trouble seems to occur in subroutine read_for_group - if a netgroup
with no members is read, the routine does not insert that empty
netgroup into the linked list of netgroups.  In the YP (NIS) case,
this will cause the routine to loop over and over trying to read
the same netgroup looking for a non-empty one.  The code changes
below move the check for an empty netgroup such that it only
affects the handling of reading and parsing the remainder of
the netgroup line.

Would appreciate it VERY much if we could get this into the release
of FreeBSD 4.3 :) :)


--- /usr/src/lib/libc/gen/getnetgrent.c	Wed Nov  3 22:16:27 1999
+++ getnetgrent.c	Tue Apr 10 18:25:08 2001
@@ -570,12 +570,14 @@
 		len = pos - spos;
 		while (*pos == ' ' || *pos == '\t')
 			pos++;
+		lp = (struct linelist *)malloc(sizeof (*lp));
+		lp->l_parsed = 0;
+		lp->l_groupname = (char *)malloc(len + 1);
+		bcopy(spos, lp->l_groupname, len);
+		*(lp->l_groupname + len) = '\0';
+
+		linep = (char *)0;
 		if (*pos != '\n' && *pos != '\0') {
-			lp = (struct linelist *)malloc(sizeof (*lp));
-			lp->l_parsed = 0;
-			lp->l_groupname = (char *)malloc(len + 1);
-			bcopy(spos, lp->l_groupname, len);
-			*(lp->l_groupname + len) = '\0';
 			len = strlen(pos);
 			olen = 0;
 
@@ -609,16 +611,17 @@
 						cont = 0;
 				}
 			} while (cont);
-			lp->l_line = linep;
-			lp->l_next = linehead;
-			linehead = lp;
-
-			/*
-			 * If this is the one we wanted, we are done.
-			 */
-			if (!strcmp(lp->l_groupname, group))
-				return (lp);
 		}
+
+		lp->l_line = linep;
+		lp->l_next = linehead;
+		linehead = lp;
+
+		/*
+		 * If this is the one we wanted, we are done.
+		 */
+		if (!strcmp(lp->l_groupname, group))
+			return (lp);
 	}
 #ifdef YP
 	/*
Comment 2 gil 2002-06-19 03:36:03 UTC
(blows into mic) -- Is anyone listening out there???

This bug that I reported has been open for over a year now.  I've
written to the FreeBSD core team, and have gotten little response
(thanks Jordan for responding, and you were a great asset to the
FreeBSD core team).

Now we're at 4.6 and even though I've provided a patch for the bug,
code to exercise the bug, and have bugged the core team about it (back
several months ago), the bug remains open and the problem still remains.

I do appreciate all the hard work that the people involved with FreeBSD
have expended.  However, I am quite frustrated by the continued lack
of response to this bug report and the recommended fix.  I don't
think that this one bug should cause me to have to take the time to
create my own sub-release of FreeBSD 4.6.

Would SOMEONE please take a look at bug misc/26486 and either
incorporate the fix into the next release (unfortunately I'm too late
for 4.6 and I'm about to deploy this on all our FreeBSD servers)...or
after some consideration, let me know that the fix isn't the correct
one.  I would like to remove the hacks I've needed to put into our
other tools in order to avoid tickling this problem.

Thanks, as always, for your consideration.

---
Gil Kloepfer - Network Support
Applied Research Laboratories : The University of Texas at Austin
(512) 835-3771 / gil@arlut.utexas.edu
Comment 3 Remko Lodder freebsd_committer freebsd_triage 2007-03-07 21:25:32 UTC
State Changed
From-To: open->feedback

Hello, Our apologies for our late reply :( Is this still relevant 
for current FreeBSD releases? If so I will handle this for you! 


Comment 4 Remko Lodder freebsd_committer freebsd_triage 2007-03-07 21:25:32 UTC
Responsible Changed
From-To: freebsd-bugs->remko

Grab the PR and make this to a good end!
Comment 5 Mark Linimon freebsd_committer freebsd_triage 2008-03-01 19:43:46 UTC
State Changed
From-To: feedback->closed

Feedback timeout (> 6 months).
Comment 6 gil 2008-03-01 20:37:18 UTC
There **WAS** feedback, but it was ignored.  See below:

On Wed, Mar 07, 2007 at 03:42:22PM -0600, Gil Kloepfer wrote:
> Remko-
> 
> Yes, it is still an issue but I'm not sure if the patch I submitted
> will still work with the current 6.x source.  Let me know if you
> need additional details.
> 
> -- 
> Gil Kloepfer - ARL Network & Unix Technology Services
> Applied Research Laboratories : The University of Texas at Austin
> (512) 835-3771 / gil@arlut.utexas.edu
> NOTE: Please send e-mail in plaintext ONLY.  HTML or RTF mail will NOT be read!
> 
> On Wed, Mar 07, 2007 at 09:26:11PM +0000, Remko Lodder wrote:
> > Synopsis: [libc] [patch] setnetgrent hangs when netgroup contains empty netgroup
> > 
> > State-Changed-From-To: open->feedback
> > State-Changed-By: remko
> > State-Changed-When: Wed Mar 7 21:25:32 UTC 2007
> > State-Changed-Why: 
> > Hello, Our apologies for our late reply :( Is this still relevant
> > for current FreeBSD releases? If so I will handle this for you!
> > 
> > 
> > Responsible-Changed-From-To: freebsd-bugs->remko
> > Responsible-Changed-By: remko
> > Responsible-Changed-When: Wed Mar 7 21:25:32 UTC 2007
> > Responsible-Changed-Why: 
> > Grab the PR and make this to a good end!
> > 
> > http://www.freebsd.org/cgi/query-pr.cgi?pr=26486
Comment 7 Mark Linimon freebsd_committer freebsd_triage 2008-03-01 23:17:26 UTC
State Changed
From-To: closed->open

Closed by mistake.
Comment 8 Remko Lodder freebsd_committer freebsd_triage 2012-09-27 08:03:17 UTC
Responsible Changed
From-To: remko->freebsd-bugs

Reassign to the group, I have held this locked way to long
Comment 9 Pedro F. Giffuni freebsd_committer freebsd_triage 2015-02-19 19:36:59 UTC
On FreeBSD 10, the test program produces this:

% ./test
1
2
4
5

(no hang).
The patch doesn't apply cleanly as the base code added a check on line
590:
			if (lp == NULL) 
				return (NULL);
Comment 10 gil 2015-02-19 20:33:50 UTC
(In reply to Pedro F. Giffuni from comment #9)

We're not running FreeBSD here any longer (sigh...) but I believe that this still indicates a problem, since the output from the test program should provide the members of the "omg-ssh-users" netgroup, but appears to stop doing any further processing as soon as it encounters an empty netgroup.

I have been away from FreeBSD for a while, but I would be willing to try to fix this bug correctly again if someone would be so kind as to commit the patch (I'll emphasize the word 'try').

At least it is no longer hanging.
Comment 11 Pedro F. Giffuni freebsd_committer freebsd_triage 2015-02-19 21:35:01 UTC
(In reply to gil from comment #10)
Thank you for checking and sorry that it took so long for someone to at least look at this.

I tracked the related commits as r235740 and r237159 and I added a reference to the PR solved by them. I cannot compromise on committing a new patch but if you do get to look at the remaining issue I will at least try to get it properly reviewed by someone who knows better than me.
Comment 12 gil 2015-02-19 21:39:02 UTC
Created attachment 153194 [details]
Updated patch to solve hang on empty netgroups for FreeBSD 10

I was able to test this again, and the FreeBSD 10.1 version still hangs on an empty netgroup with NIS (note, NIS is necessary for this to be a problem).

The attached patch was based on my original patch, and has not been thoroughly tested or peer-reviewed.  I recommend that whoever commits the patch please look through and verify that my logic is still correct.
Comment 13 Pedro F. Giffuni freebsd_committer freebsd_triage 2015-02-19 21:47:48 UTC
(In reply to gil from comment #12)
OK.. I don't have the problem because I am not running NIS.

I will get someone that does to review. Thank you!
Comment 14 Jilles Tjoelker freebsd_committer freebsd_triage 2015-02-24 22:03:26 UTC
This looks like it may work, but changes some functional behaviour apart from fixing the infinite loop: a netgroup without members is now returned from read_for_group() instead of skipped.

An alternative would be to add a check after for a netgroup without members after if (yp_match(_netgr_yp_domain, "netgroup" ..., and return NULL if so.

Apart from the global-variable happiness of this code (which does not directly break it but makes it thread-unsafe and confusing), I notice another bug: netgroups from NIS must fit in LINSIZ, while netgroups from /etc/netgroup can be as long as fits in memory using continuation lines.
Comment 15 Pedro F. Giffuni freebsd_committer freebsd_triage 2015-02-24 22:13:33 UTC
Assign to Mark as he has some nearby changes in the works. (I will continue in the CC List).
Comment 16 Marcelo Araujo freebsd_committer freebsd_triage 2015-11-17 02:30:18 UTC
Mark, do you have any patch that I could take a look?
Comment 17 commit-hook freebsd_committer freebsd_triage 2016-06-09 01:11:53 UTC
A commit references this bug:

Author: markj
Date: Thu Jun  9 01:11:48 UTC 2016
New revision: 301710
URL: https://svnweb.freebsd.org/changeset/base/301710

Log:
  Fix an infinite loop in setnetgrent(3) with NIS netgroups.

  Handle an empty result from yp_match() by returning NULL, which is
  consistent with the handling of an empty netgroup in /etc/netgroup.
  setnetgrent(3) has no return value, so there is no particular need to
  distinguish this case from an error.

  PR:		26486
  MFC after:	2 weeks

Changes:
  head/lib/libc/gen/getnetgrent.c
Comment 18 Mark Johnston freebsd_committer freebsd_triage 2016-08-02 21:18:45 UTC
Fixed in stable/10 and 9 with r303680 and r303681, respectively.