Autofs and NIS+ bug

Autofs and NIS+ bug

[Peter C. Norton]

Greetings list,

I've been working on an issue with the lookup_nisplus.c that runs
headfirst into a bug in the linux glibc's NIS+ code.

First, let me start by saying that I know that NIS+ is a dinosaur, but
I'm stuck supporting it, so by using some code from the linux nis+
tools, I've found a way to fix the issue by using this code and
patching lookup_nisplus.c.

Background:

NIS+ is an update to the venerable NIS. Sun created it as a system
that would allow for heirarchical domains. Whereas NIS/YP required a
flat single domain, and one domain couldn't speak to another, NIS+ has
heirarchy and visibility between domains that are in the same
heirarchy. This visibility extends to two features that allow a table
in one directory to actually either include data that exists in
another table (even one in another domain). These are called search
paths. The second mechanism is called a link, and like a symbolic link
on a filesystem, it points a table in one directory to another table
in its entirety (paths include another table, links do not allow the
mixture of local and remote data).

The Problem:

The normal, easy way, to perform a lookup using NIS+ involves a few
simple calls:

calling nis_lookup() with the argument of a string table_name, and a
list of options which can specify performing resolution of the domain,
following links, following paths, and other behaviours.

Then, using the table name returned by nis_lookup (from which the
linked-to table name can also be gotten) you pass a string that is the
lookup to the call nis_list(table_name, flags, callback(), [separator]).

The result from the nis_list should be useable, via the callback, as
an argument to the nis_print_object() call. When the looking being
performed is not an indexed lookup (a lookup that does not have the
equivelant of a SQL "WHERE" clauses) this works. When the looking is
an indexed lookup, the call fails by causing a segfault somewhere deep
in the c library. Trying to debug this has led me to having
nonsensical debugging info - I assume due to optimization on my test
platform (Red Hat Enterprise Linux 3).

In trying to unwind this, I used the results of the nis_lookup to get
the name of the actual table being referred to, and tried to call
nis_list on the table directly, but the indexed lookup crashed yet
again.

Resolution:

After butting my head against this, I used code from the linux NIS+
tools, which instead of using the nis_print* functions seems to access
the individual elements of the returned array, and prints them itself.

Doing this from within a callback has caused a tiny bit of ugliness in
the code, but I can confirm that where before I had businesses that
could not use the automounter in some parts of their NIS+ environment,
now they can.

So, I'd like to know if I can submit this code for the review of the
list to be included. I am doing this with two goals in mind: 1) if
there is anyone else out there who is in the same unfortunate support
situation as my company (a very large one that is trying to move away
from NIS+, but it is slooooooow going) I'd like to save them the
effort. 2) I'd like to encourage my linux vendor (Red Hat) to get this
included in their internal package by making it a mainstream part of
the automounter until NIS+ finally gets a bullet in the head. I know
Jeff has done a lot of good work here and is agressively fixing autofs
in RHEL and Fedora, but from the management end this sort of task can
be difficult.

So let me know if in principle this kind of patch could be reviewed,
and I'll pass it along, with a minimal description of how to reproduce
a test case... if anyone is interested.

Thanks,

-Peter

-- 
The 5 year plan:
In five years we'll make up another plan.
Or just re-use this one.

Re: Autofs and NIS+ bug

[Jeff Moyer]

==> Regarding [autofs] Autofs and NIS+ bug; "Peter C. Norton" <spacey-autofs@lenin.net> adds:

spacey-autofs> Greetings list, I've been working on an issue with the
spacey-autofs> lookup_nisplus.c that runs headfirst into a bug in the linux
spacey-autofs> glibc's NIS+ code.

[snip]

spacey-autofs> So let me know if in principle this kind of patch could be
spacey-autofs> reviewed, and I'll pass it along, with a minimal description
spacey-autofs> of how to reproduce a test case... if anyone is interested.

Of course you should send the patch!  Also, please don't skimp on the
description of how to reproduce.  If I can't reliably reproduce the problem
and verify the fix, I have a hard time justifying the fix to management!

-Jeff

Re: Autofs and NIS+ bug

[raven]

On Fri, 3 Jun 2005, Peter C. Norton wrote:

> Greetings list,
> 
> I've been working on an issue with the lookup_nisplus.c that runs
> headfirst into a bug in the linux glibc's NIS+ code.
> 
> First, let me start by saying that I know that NIS+ is a dinosaur, but
> I'm stuck supporting it, so by using some code from the linux nis+
> tools, I've found a way to fix the issue by using this code and
> patching lookup_nisplus.c.
> 
> Background:
> 
> NIS+ is an update to the venerable NIS. Sun created it as a system
> that would allow for heirarchical domains. Whereas NIS/YP required a
> flat single domain, and one domain couldn't speak to another, NIS+ has
> heirarchy and visibility between domains that are in the same
> heirarchy. This visibility extends to two features that allow a table
> in one directory to actually either include data that exists in
> another table (even one in another domain). These are called search
> paths. The second mechanism is called a link, and like a symbolic link
> on a filesystem, it points a table in one directory to another table
> in its entirety (paths include another table, links do not allow the
> mixture of local and remote data).
> 
> The Problem:
> 
> The normal, easy way, to perform a lookup using NIS+ involves a few
> simple calls:
> 
> calling nis_lookup() with the argument of a string table_name, and a
> list of options which can specify performing resolution of the domain,
> following links, following paths, and other behaviours.
> 
> Then, using the table name returned by nis_lookup (from which the
> linked-to table name can also be gotten) you pass a string that is the
> lookup to the call nis_list(table_name, flags, callback(), [separator]).
> 
> The result from the nis_list should be useable, via the callback, as
> an argument to the nis_print_object() call. When the looking being
> performed is not an indexed lookup (a lookup that does not have the
> equivelant of a SQL "WHERE" clauses) this works. When the looking is
> an indexed lookup, the call fails by causing a segfault somewhere deep
> in the c library. Trying to debug this has led me to having
> nonsensical debugging info - I assume due to optimization on my test
> platform (Red Hat Enterprise Linux 3).
> 
> In trying to unwind this, I used the results of the nis_lookup to get
> the name of the actual table being referred to, and tried to call
> nis_list on the table directly, but the indexed lookup crashed yet
> again.
> 
> Resolution:
> 
> After butting my head against this, I used code from the linux NIS+
> tools, which instead of using the nis_print* functions seems to access
> the individual elements of the returned array, and prints them itself.
> 
> Doing this from within a callback has caused a tiny bit of ugliness in
> the code, but I can confirm that where before I had businesses that
> could not use the automounter in some parts of their NIS+ environment,
> now they can.
> 
> So, I'd like to know if I can submit this code for the review of the
> list to be included. I am doing this with two goals in mind: 1) if
> there is anyone else out there who is in the same unfortunate support
> situation as my company (a very large one that is trying to move away
> from NIS+, but it is slooooooow going) I'd like to save them the
> effort. 2) I'd like to encourage my linux vendor (Red Hat) to get this
> included in their internal package by making it a mainstream part of
> the automounter until NIS+ finally gets a bullet in the head. I know
> Jeff has done a lot of good work here and is agressively fixing autofs
> in RHEL and Fedora, but from the management end this sort of task can
> be difficult.

Off course you can submit the code.
Unforetuneatly, I don't have a test environment and I'm not familiar with 
the nis+ api so I'll have to rely on you to test the result.

> 
> So let me know if in principle this kind of patch could be reviewed,
> and I'll pass it along, with a minimal description of how to reproduce
> a test case... if anyone is interested.

I'm happy to include anything that helps.
Of course if it is hard to merge or gets out of date (for whatever 
reason) before I can look at it then it gets harder.

Generally I expect people to maintain there patches for as long as it 
might take to get them merged.

Ian

Re: Autofs and NIS+ bug

[Peter C. Norton]

[-- Attachment #1: Type: text/plain, Size: 1081 bytes --]

On Sat, Jun 04, 2005 at 01:24:21PM +0800, raven@themaw.net wrote:
> 
> Off course you can submit the code.
> Unforetuneatly, I don't have a test environment and I'm not familiar with 
> the nis+ api so I'll have to rely on you to test the result.

I thought I'd sent the patch to the list, but I think I may only have
sent it to Jeff Moyer
 
> > So let me know if in principle this kind of patch could be reviewed,
> > and I'll pass it along, with a minimal description of how to reproduce
> > a test case... if anyone is interested.
> 
> I'm happy to include anything that helps.
> Of course if it is hard to merge or gets out of date (for whatever 
> reason) before I can look at it then it gets harder.
> 
> Generally I expect people to maintain there patches for as long as it 
> might take to get them merged.

It should be timeless, because NIS+ is going away.  I am maintaining
the patch internally, so that's not a big deal.

I'm attaching the patch again with this message.

-Peter

-- 
The 5 year plan:
In five years we'll make up another plan.
Or just re-use this one.


[-- Attachment #2: lookup_nisplus.patch --]
[-- Type: text/plain, Size: 9780 bytes --]

--- autofs-4.1.3/modules/lookup_nisplus.c	2005-05-13 12:35:29.000000000 -0400
+++ /home/pn18117/tmp/lookup_nisplus-4.c	2005-05-13 12:38:51.000000000 -0400
@@ -1,4 +1,4 @@
-#ident "$Id: lookup_nisplus.c,v 1.3 2004/01/29 16:01:22 raven Exp $"
+#ident "$Id: lookup_nisplus.c,v 1.2 1998/03/29 02:03:06 hpa Exp $"
 /*
  * lookup_nisplus.c
  *
@@ -24,88 +24,279 @@
 
 #define MODPREFIX "lookup(nisplus): "
 
-struct lookup_context {
+struct lookup_context
+{
 	const char *domainname;
 	const char *mapname;
 	struct parse_mod *parse;
 };
 
+#define STORAGE_BUF_LEN 2048
+static char storage_buf[STORAGE_BUF_LEN] = "";
+
 int lookup_version = AUTOFS_LOOKUP_VERSION;	/* Required by protocol */
 
-int lookup_init(const char *mapfmt, int argc, const char *const *argv, void **context)
+int
+lookup_init (const char *mapfmt, int argc, const char *const *argv,
+	     void **context)
 {
 	struct lookup_context *ctxt;
 
-	if (!(*context = ctxt = malloc(sizeof(struct lookup_context)))) {
-		crit(MODPREFIX "%m");
+	if (!(*context = ctxt = malloc (sizeof (struct lookup_context)))) {
+		syslog (LOG_CRIT, MODPREFIX "%m");
 		return 1;
 	}
 
 	if (argc < 1) {
-		crit(MODPREFIX "No map name");
+		syslog (LOG_CRIT, MODPREFIX "No map name");
 		return 1;
 	}
 	ctxt->mapname = argv[0];
 
-	/* 
-	 * nis_local_directory () returns a pointer to a static buffer.
-	 * We don't need to copy or free it.
-	 */
-	ctxt->domainname = nis_local_directory();
+	/* nis_local_directory () returns a pointer to a static buffer.
+	   We don't need to copy or free it. */
+	ctxt->domainname = nis_local_directory ();
 
 	if (!mapfmt)
 		mapfmt = MAPFMT_DEFAULT;
 
-	return !(ctxt->parse = open_parse(mapfmt, MODPREFIX, argc - 1, argv + 1));
+	return !(ctxt->parse =
+		 open_parse (mapfmt, MODPREFIX, argc - 1, argv + 1));
+}
+
+static int
+insert_to_buffer (const_nis_name name __attribute__ ((unused)),
+		  const nis_object * obj, const void *udata)
+{
+	const char *separator = udata;
+	u_int j;
+	char *buf = storage_buf;
+	int raw_binary = 0;
+	int buf_len = 0;
+
+	syslog (LOG_NOTICE, MODPREFIX "inserting to buffer, whatever");
+
+	for (j = 1; j < obj->EN_data.en_cols.en_cols_len; ++j) {
+		if (ENTRY_VAL (obj, j) != NULL) {
+			if ((obj->EN_data.en_cols.en_cols_val->
+			     ec_flags & EN_BINARY)
+			    && !raw_binary)
+				fputs ("*BINARY*", stdout);
+			else {
+				if ((buf_len + (int) ENTRY_LEN (obj, j)) >
+				    STORAGE_BUF_LEN) {
+					syslog (LOG_NOTICE,
+						MODPREFIX
+						"Trying to grow storage_buf beyond the limit of %d bytes while looking up an entry. Giving up",
+						STORAGE_BUF_LEN);
+					return -1;
+				}
+				sprintf (buf, "%.*s",
+					 (int) ENTRY_LEN (obj, j),
+					 ENTRY_VAL (obj, j));
+				if ((strlen (buf) +
+				     ((int) ENTRY_LEN (obj, j)) >
+				     STORAGE_BUF_LEN)) {
+					return -1;	/* XXX I don't know what the callback
+							 * should return to indicate an error, so
+							 * I'm going to punt here. -PN
+							 * <me@my_big_company>
+							 */
+				}
+				buf += ((int) ENTRY_LEN (obj, j) - 1);
+				buf_len += ((int) ENTRY_LEN (obj, j) - 1);
+			}
+		}
+		if (j + 1 < obj->EN_data.en_cols.en_cols_len) {
+			sprintf (buf, "%c", *separator);
+			buf++;
+		}
+	}
+	syslog (LOG_NOTICE, MODPREFIX "buf is %s", storage_buf);
+	// fputs ("\n", stdout);
+	return 0;
 }
 
-int lookup_ghost(const char *root, int ghost, time_t age, void *context)
+
+
+int
+lookup_mount (const char *root, const char *name, int name_len, void *context)
 {
-	return LKP_NOTSUP;
+	struct lookup_context *ctxt = (struct lookup_context *) context;
+	nis_result *res = NULL;
+	char separator = '\t';
+	int display_header = 0;
+	int print_object = 0;
+	int new_sep = 0;
+	int flags = EXPAND_NAME;
+	int i, result = 0;
+	char *mapname;
+	char my_tblname[2048];
+	char *tablename;
+
+	flags |= FOLLOW_LINKS;
+	flags |= FOLLOW_PATH;
+
+	syslog (LOG_NOTICE, MODPREFIX "looking up %s", name);
+
+	// sprintf(my_tblname, "[key=%s],%s.org_dir.%s", name, ctxt->mapname, ctxt->domainname);
+	sprintf (my_tblname, "[key=%s],%s.org_dir", name, ctxt->mapname);
+	tablename = strchr (my_tblname, ']');
+
+
+	syslog (LOG_NOTICE, MODPREFIX "looking up %s", my_tblname);
+
+
+	// syslog(LOG_NOTICE, MODPREFIX "tablename is: %s", tablename);
+	// syslog("tablename is %s\n", tablename);
+	if (tablename != NULL)
+		tablename += 2;
+	else
+		tablename = my_tblname;
+
+
+	syslog (LOG_NOTICE, MODPREFIX "now tablename is %s", tablename);
+	// printf("now tablename is %s\n", tablename);
+	result = 0;
+	res = nis_lookup (tablename, flags);
+	// syslog(LOG_NOTICE, MODPREFIX "resolved, buf is %s", storage_buf);
+
+	if (res == NULL ||
+	    (res->status != NIS_SUCCESS && res->status != NIS_S_SUCCESS)) {
+		// syslog(LOG_NOTICE, MODPREFIX "part 1: Storage buf ends up being: %s", storage_buf); 
+		if (res == NULL) {
+			fputs ("Out of memory!\n", stderr);
+			return -1;
+		}
+		else {
+			syslog (LOG_NOTICE,
+				MODPREFIX "lookup for %s failed: %s", name,
+				nis_sperrno (res->status));
+			// nis_perror (res->status, tablename);
+		}
+		nis_freeresult (res);
+		result = 1;
+		return -1;
+	}
+
+	/* If we have an TABLE_OBJ and don't print the object structure, get
+	   the table contents. */
+	if (__type_of (NIS_RES_OBJECT (res)) == NIS_TABLE_OBJ &&
+	    (!print_object || my_tblname[0] == '['))
+	{
+		char *buf = alloca (strlen (my_tblname) + 5);
+
+		if (buf == NULL) {
+			syslog (LOG_NOTICE, MODPREFIX "Out of memory!\n",
+				stderr);
+			return -1;
+		}
+
+		if (!new_sep && !print_object) {
+			/* Get the separator character */
+			separator = NIS_RES_OBJECT (res)->TA_data.ta_sep;
+		}
+		/* Get the contents of the table */
+		if (my_tblname[0] == '[')	/* Indexed names! */
+			strcpy (buf, my_tblname);
+		else
+			sprintf (buf, "[],%s", my_tblname);
+
+		res = nis_list (buf, flags, insert_to_buffer,
+				(void *) &separator);
+
+		// If there's nothing in the storage_buf (starts with a null char), then
+		// we need to do the lookup one more time for the "*" entry.
+		// -PN
+		syslog (LOG_NOTICE, MODPREFIX "buf is: %x", *storage_buf);
+		if (storage_buf[0] == '\0') {
+			// sprintf(my_tblname, "[key=*],%s.org_dir.%s", ctxt->mapname, ctxt->domainname);
+			sprintf (my_tblname, "[key=*],%s.org_dir",
+				 ctxt->mapname);
+			syslog (LOG_NOTICE,
+				MODPREFIX
+				"Got a fall-through, looking for %s",
+				my_tblname);
+			strcpy (buf, my_tblname);
+			res = nis_list (buf, flags, insert_to_buffer,
+					(void *) &separator);
+		}
+
+		if (res == NULL ||
+		    (res->status != NIS_SUCCESS
+		     && res->status != NIS_CBRESULTS)) {
+			if (res == NULL)
+				syslog (LOG_NOTICE,
+					MODPREFIX "Out of memory!");
+			else {
+				if (res->status == NIS_PARTIAL)
+					return -1;
+				nis_perror (res->status, my_tblname);
+			}
+			nis_freeresult (res);
+			result = 1;
+			return -1;
+		}
+	}
+
+	syslog (LOG_NOTICE,
+		MODPREFIX "part 2: Storage buf ends up being: %s\n",
+		storage_buf);
+	result = ctxt->parse->parse_mount (root, name, name_len, storage_buf,
+					   ctxt->parse->context);
+
+	syslog (LOG_NOTICE, MODPREFIX "%s -> %s", name, storage_buf);
+
+	return result;
 }
 
-int lookup_mount(const char *root, const char *name, int name_len, void *context)
+
+int
+lookup_mount_dist (const char *root, const char *name, int name_len,
+		   void *context)
 {
 	struct lookup_context *ctxt = (struct lookup_context *) context;
-	char tablename[strlen(name) + strlen(ctxt->mapname) +
-		       strlen(ctxt->domainname) + 20];
+	char tablename[strlen (name) + strlen (ctxt->mapname) +
+		       strlen (ctxt->domainname) + 20];
 	nis_result *result;
 	int rv;
 
-	debug(MODPREFIX "looking up %s", name);
+	syslog (LOG_DEBUG, MODPREFIX "looking up %s", name);
 
-	sprintf(tablename, "[key=%s],%s.org_dir.%s", name, ctxt->mapname,
-		ctxt->domainname);
+	sprintf (tablename, "[key=%s],%s.org_dir.%s", name, ctxt->mapname,
+		 ctxt->domainname);
 
-	result = nis_list(tablename, FOLLOW_PATH | FOLLOW_LINKS, NULL, NULL);
+	result = nis_list (tablename, FOLLOW_PATH | FOLLOW_LINKS, NULL, NULL);
 	if (result->status != NIS_SUCCESS && result->status != NIS_S_SUCCESS) {
 		/* Try to get the "*" entry if there is one - note that we *don't*
 		   modify "name" so & -> the name we used, not "*" */
-		sprintf(tablename, "[key=*],%s.org_dir.%s", ctxt->mapname,
-			ctxt->domainname);
-		result = nis_list(tablename, FOLLOW_PATH | FOLLOW_LINKS, NULL, NULL);
+		sprintf (tablename, "[key=*],%s.org_dir.%s", ctxt->mapname,
+			 ctxt->domainname);
+		result = nis_list (tablename, FOLLOW_PATH | FOLLOW_LINKS,
+				   NULL, NULL);
 	}
 	if (result->status != NIS_SUCCESS && result->status != NIS_S_SUCCESS) {
-		crit(MODPREFIX "lookup for %s failed: %s", name,
-		       nis_sperrno(result->status));
+		syslog (LOG_NOTICE, MODPREFIX "lookup for %s failed: %s",
+			name, nis_sperrno (result->status));
 		return 1;
 	}
 
-	debug(MODPREFIX "%s -> %s", name,
-	       NIS_RES_OBJECT(result)->EN_data.en_cols.en_cols_val[1].ec_value.
-	       ec_value_val);
-
-	rv = ctxt->parse->parse_mount(root, name, name_len,
-				      NIS_RES_OBJECT(result)->EN_data.en_cols.
-				      en_cols_val[1].ec_value.ec_value_val,
-				      ctxt->parse->context);
+	syslog (LOG_DEBUG, MODPREFIX "%s -> %s", name,
+		NIS_RES_OBJECT (result)->EN_data.en_cols.en_cols_val[1].
+		ec_value.ec_value_val);
+
+	rv = ctxt->parse->parse_mount (root, name, name_len,
+				       NIS_RES_OBJECT (result)->EN_data.
+				       en_cols.en_cols_val[1].ec_value.
+				       ec_value_val, ctxt->parse->context);
 	return rv;
 }
 
-int lookup_done(void *context)
+int
+lookup_done (void *context)
 {
 	struct lookup_context *ctxt = (struct lookup_context *) context;
-	int rv = close_parse(ctxt->parse);
-	free(ctxt);
+	int rv = close_parse (ctxt->parse);
+	free (ctxt);
 	return rv;
 }

[-- Attachment #3: Type: text/plain, Size: 140 bytes --]

_______________________________________________
autofs mailing list
autofs@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/autofs