[PATCH] Only lookup uid/gid when applying rules

[PATCH] Only lookup uid/gid when applying rules

[Roy Marples]

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

Hi List

Attached is a patch that stops udev from doing a uid/gid lookup unless it is 
actually going to use the rule.

This is important as udev ships with rules for user/group names that may not 
exist in /etc/passwd. Normally this would not be a problem, but if a system 
is configured for LDAP, NIS or some other off line system at boot time then 
things get very nasty.

See Gentoo bug #99564 for futher commentary on this
http://bugs.gentoo.org/show_bug.cgi?id=99564

Thanks

-- 
Roy Marples <uberlord@gentoo.org>
Gentoo/Linux Developer (baselayout, networking)

[-- Attachment #2: udev-nolookup.patch --]
[-- Type: text/x-diff, Size: 3850 bytes --]

diff -u udev-094.orig/udev.c udev-094/udev.c
--- udev-094.orig/udev.c	2006-07-07 14:39:34.000000000 +0100
+++ udev-094/udev.c	2006-07-07 14:41:39.000000000 +0100
@@ -128,7 +128,7 @@
 	}
 
 	sysfs_init();
-	udev_rules_init(&rules, 0);
+	udev_rules_init(&rules);
 
 	dev = sysfs_device_get(devpath);
 	if (dev == NULL) {
diff -u udev-094.orig/udevd.c udev-094/udevd.c
--- udev-094.orig/udevd.c	2006-07-07 14:39:34.000000000 +0100
+++ udev-094/udevd.c	2006-07-07 14:41:56.000000000 +0100
@@ -908,7 +908,7 @@
 
 	/* parse the rules and keep it in memory */
 	sysfs_init();
-	udev_rules_init(&rules, 1);
+	udev_rules_init(&rules);
 
 	export_initial_seqnum();
 
@@ -1088,7 +1088,7 @@
 		if (reload_config) {
 			reload_config = 0;
 			udev_rules_cleanup(&rules);
-			udev_rules_init(&rules, 1);
+			udev_rules_init(&rules);
 		}
 
 		/* forked child has returned */
diff -u udev-094.orig/udev_rules.h udev-094/udev_rules.h
--- udev-094.orig/udev_rules.h	2006-07-07 14:39:34.000000000 +0100
+++ udev-094/udev_rules.h	2006-07-07 14:41:27.000000000 +0100
@@ -98,10 +98,9 @@
 	char *buf;
 	size_t bufsize;
 	size_t current;
-	int resolve_names;
 };
 
-extern int udev_rules_init(struct udev_rules *rules, int resolve_names);
+extern int udev_rules_init(struct udev_rules *rules);
 extern void udev_rules_cleanup(struct udev_rules *rules);
 
 extern void udev_rules_iter_init(struct udev_rules *rules);
diff -u udev-094.orig/udev_rules_parse.c udev-094/udev_rules_parse.c
--- udev-094.orig/udev_rules_parse.c	2006-07-07 14:39:34.000000000 +0100
+++ udev-094/udev_rules_parse.c	2006-07-07 14:41:00.000000000 +0100
@@ -473,38 +473,12 @@
 
 		if (strcasecmp(key, "OWNER") == 0) {
 			valid = 1;
-			if (rules->resolve_names && (!strchr(value, '$') && !strchr(value, '%'))) {
-				char *endptr;
-				strtoul(value, &endptr, 10);
-				if (endptr[0] != '\0') {
-					char owner[32];
-					uid_t uid = lookup_user(value);
-					dbg("replacing username='%s' by id=%i", value, uid);
-					sprintf(owner, "%u", (unsigned int) uid);
-					add_rule_key(rule, &rule->owner, operation, owner);
-					continue;
-				}
-			}
-
 			add_rule_key(rule, &rule->owner, operation, value);
 			continue;
 		}
 
 		if (strcasecmp(key, "GROUP") == 0) {
 			valid = 1;
-			if (rules->resolve_names && (!strchr(value, '$') && !strchr(value, '%'))) {
-				char *endptr;
-				strtoul(value, &endptr, 10);
-				if (endptr[0] != '\0') {
-					char group[32];
-					gid_t gid = lookup_group(value);
-					dbg("replacing groupname='%s' by id=%i", value, gid);
-					sprintf(group, "%u", (unsigned int) gid);
-					add_rule_key(rule, &rule->group, operation, group);
-					continue;
-				}
-			}
-
 			add_rule_key(rule, &rule->group, operation, value);
 			continue;
 		}
@@ -637,13 +611,12 @@
 	return retval;
 }
 
-int udev_rules_init(struct udev_rules *rules, int resolve_names)
+int udev_rules_init(struct udev_rules *rules)
 {
 	struct stat stats;
 	int retval;
 
 	memset(rules, 0x00, sizeof(struct udev_rules));
-	rules->resolve_names = resolve_names;
 
 	/* parse rules file or all matching files in directory */
 	if (stat(udev_rules_filename, &stats) != 0)
diff -u udev-094.orig/udevstart.c udev-094/udevstart.c
--- udev-094.orig/udevstart.c	2006-07-07 14:39:34.000000000 +0100
+++ udev-094/udevstart.c	2006-07-07 14:42:25.000000000 +0100
@@ -361,7 +361,7 @@
 	alarm(UDEV_ALARM_TIMEOUT);
 
 	sysfs_init();
-	udev_rules_init(&rules, 1);
+	udev_rules_init(&rules);
 
 	udev_scan_class(&device_list);
 	udev_scan_block(&device_list);
diff -u udev-094.orig/udevtest.c udev-094/udevtest.c
--- udev-094.orig/udevtest.c	2006-07-07 14:39:34.000000000 +0100
+++ udev-094/udevtest.c	2006-07-07 14:42:11.000000000 +0100
@@ -83,7 +83,7 @@
 			devpath = argv[1];
 
 	sysfs_init();
-	udev_rules_init(&rules, 0);
+	udev_rules_init(&rules);
 
 	dev = sysfs_device_get(devpath);
 	if (dev == NULL) {

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

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

[-- Attachment #4: Type: text/plain, Size: 226 bytes --]

_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [PATCH] Only lookup uid/gid when applying rules

[Kay Sievers]

On Tue, 2006-08-01 at 11:35 +0100, Roy Marples wrote:
> Attached is a patch that stops udev from doing a uid/gid lookup unless it is 
> actually going to use the rule.

Nope, that doesn't really help, we want to to resolve the names once on
udevd startup and not from every event process again and again.

You may introduce a flag to udevd that tells to use only the built-in
passwd parser. And also to wait only once for a timeout of getpwnam()
and then automatically switch to the built-in parser. A later
reload-rules event would try to use getpwnam() again, so you can update
udevd when your network is available.

> This is important as udev ships with rules for user/group names that may not 
> exist in /etc/passwd. Normally this would not be a problem, but if a system 
> is configured for LDAP, NIS or some other off line system at boot time then 
> things get very nasty.

Right, this should be fixed, but in a different way.

Thanks,
Kay


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CIDÞVDEV
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [PATCH] Only lookup uid/gid when applying rules

[Roy Marples]

On Tuesday 01 August 2006 13:35, Kay Sievers wrote:
> On Tue, 2006-08-01 at 11:35 +0100, Roy Marples wrote:
> > Attached is a patch that stops udev from doing a uid/gid lookup unless it
> > is actually going to use the rule.
>
> Nope, that doesn't really help, we want to to resolve the names once on
> udevd startup and not from every event process again and again.
>
> You may introduce a flag to udevd that tells to use only the built-in
> passwd parser. And also to wait only once for a timeout of getpwnam()
> and then automatically switch to the built-in parser. A later
> reload-rules event would try to use getpwnam() again, so you can update
> udevd when your network is available.

How about we store each name/number mapping in an array which is checked at 
each event processed before calling getpwnam()? Seems that would solve both 
issues.

Of course the array would be disposed of when the rules are reloaded.

Thanks

-- 
Roy Marples <uberlord@gentoo.org>
Gentoo/Linux Developer (baselayout, networking)

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CIDÞVDEV
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [PATCH] Only lookup uid/gid when applying rules

[Kay Sievers]

On Tue, 2006-08-01 at 13:54 +0100, Roy Marples wrote:
> On Tuesday 01 August 2006 13:35, Kay Sievers wrote:
> > On Tue, 2006-08-01 at 11:35 +0100, Roy Marples wrote:
> > > Attached is a patch that stops udev from doing a uid/gid lookup unless it
> > > is actually going to use the rule.
> >
> > Nope, that doesn't really help, we want to to resolve the names once on
> > udevd startup and not from every event process again and again.
> >
> > You may introduce a flag to udevd that tells to use only the built-in
> > passwd parser. And also to wait only once for a timeout of getpwnam()
> > and then automatically switch to the built-in parser. A later
> > reload-rules event would try to use getpwnam() again, so you can update
> > udevd when your network is available.
> 
> How about we store each name/number mapping in an array which is checked at 
> each event processed before calling getpwnam()? Seems that would solve both 
> issues.

How does that solve the problem, that you need to resolve a name when a
rule gets applied, but you will hang in getpwnam() cause of  the system
config?
Such an array will only reduce the number of lookups for the _same_
name, but not solve the problem, right?

Kay


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CIDÞVDEV
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [PATCH] Only lookup uid/gid when applying rules

[Roy Marples]

On Tuesday 01 August 2006 14:12, Kay Sievers wrote:
> How does that solve the problem, that you need to resolve a name when a
> rule gets applied, but you will hang in getpwnam() cause of  the system
> config?
> Such an array will only reduce the number of lookups for the _same_
> name, but not solve the problem, right?

OK, it doesn't really solve the problem. The issue as I see it is that every 
user/group name is looked up for each rule before events are processed.

This is bad for reasons explained earlier.

So we need to lookup the user/group at rule application time. We can 
optionally store it back in the rule or in an a id/name array.

This solves the problem of a correctly configured LDAP server who does not 
have the user/group "tss" in any nss DB and yet exists in a udev rule that 
will never be applied.

This does not solve the problem of a poorly configured LDAP/NIS/udev system 
where the user did not add tss to /etc/{passwd,group}, but the patch was not 
expected to either ;)

Thanks

-- 
Roy Marples <uberlord@gentoo.org>
Gentoo/Linux Developer (baselayout, networking)

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CIDÞVDEV
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [PATCH] Only lookup uid/gid when applying rules

[Roy Marples]

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

On Tuesday 01 August 2006 13:35, you wrote:
> On Tue, 2006-08-01 at 11:35 +0100, Roy Marples wrote:
> > Attached is a patch that stops udev from doing a uid/gid lookup unless it
> > is actually going to use the rule.
>
> Nope, that doesn't really help, we want to to resolve the names once on
> udevd startup and not from every event process again and again.

OK, this patch again removes the uid/gid lookup when loading rules as it's the 
root cause of the problem. I also don't see a flag being of any benefit.

However, this patch stores the looked up uid/gid back in the rule for future 
event processes on the same rule. So, this should also make us more efficient 
by only looking up the uid/gid for each rule when it's need, but only once 
per rule.

Hopefully this satisfies your criteria :)

NOTE: I will be unable to test this against an LDAP server with the actual 
issue in question until Monday/tuesday as it's a work server, but I thought 
I'd post it here still for review.

Thanks

-- 
Roy Marples <uberlord@gentoo.org>
Gentoo/Linux Developer (baselayout, networking)

[-- Attachment #2: udev-lookup.patch --]
[-- Type: text/x-diff, Size: 4831 bytes --]

diff -u udev-096.orig/udev.c udev-096/udev.c
--- udev-096.orig/udev.c	2006-08-05 00:16:20.000000000 +0100
+++ udev-096/udev.c	2006-08-05 00:17:11.000000000 +0100
@@ -128,7 +128,7 @@
 	}
 
 	sysfs_init();
-	udev_rules_init(&rules, 0);
+	udev_rules_init(&rules);
 
 	dev = sysfs_device_get(devpath);
 	if (dev == NULL) {
diff -u udev-096.orig/udevd.c udev-096/udevd.c
--- udev-096.orig/udevd.c	2006-08-05 00:16:20.000000000 +0100
+++ udev-096/udevd.c	2006-08-05 00:17:11.000000000 +0100
@@ -908,7 +908,7 @@
 
 	/* parse the rules and keep it in memory */
 	sysfs_init();
-	udev_rules_init(&rules, 1);
+	udev_rules_init(&rules);
 
 	export_initial_seqnum();
 
@@ -1088,7 +1088,7 @@
 		if (reload_config) {
 			reload_config = 0;
 			udev_rules_cleanup(&rules);
-			udev_rules_init(&rules, 1);
+			udev_rules_init(&rules);
 		}
 
 		/* forked child has returned */
diff -u udev-096.orig/udev_device.c udev-096/udev_device.c
--- udev-096.orig/udev_device.c	2006-08-05 00:16:20.000000000 +0100
+++ udev-096/udev_device.c	2006-08-05 00:17:11.000000000 +0100
@@ -162,6 +162,27 @@
 			goto exit;
 		}
 
+		/* If we don't have owner and group id's, look them up now and
+		   store them back in the rule for other devices. */
+		unsigned long id;
+		char *endptr;
+		uid_t uid = 0;
+		gid_t gid = 0;
+
+		id = strtoul(udev->owner, &endptr, 10);
+		if (endptr[0] != '\0') {
+			if (strcmp(udev->owner, "root") != 0)
+				uid = lookup_user(udev->owner);
+			sprintf(udev->owner, "%u", (unsigned int) uid);
+		}
+
+		id = strtoul(udev->group, &endptr, 10);
+		if (endptr[0] != '\0') {
+			if (strcmp(udev->group, "root") != 0)
+				gid = lookup_group(udev->group);
+			sprintf(udev->group, "%u", (unsigned int) gid);
+		}
+
 		/* read current database entry, we may want to cleanup symlinks */
 		udev_old = udev_device_init();
 		if (udev_old != NULL) {
diff -u udev-096.orig/udev_rules.h udev-096/udev_rules.h
--- udev-096.orig/udev_rules.h	2006-08-05 00:16:20.000000000 +0100
+++ udev-096/udev_rules.h	2006-08-05 00:17:11.000000000 +0100
@@ -98,10 +98,9 @@
 	char *buf;
 	size_t bufsize;
 	size_t current;
-	int resolve_names;
 };
 
-extern int udev_rules_init(struct udev_rules *rules, int resolve_names);
+extern int udev_rules_init(struct udev_rules *rules);
 extern void udev_rules_cleanup(struct udev_rules *rules);
 
 extern void udev_rules_iter_init(struct udev_rules *rules);
diff -u udev-096.orig/udev_rules_parse.c udev-096/udev_rules_parse.c
--- udev-096.orig/udev_rules_parse.c	2006-08-05 00:16:20.000000000 +0100
+++ udev-096/udev_rules_parse.c	2006-08-05 00:17:11.000000000 +0100
@@ -473,38 +473,12 @@
 
 		if (strcasecmp(key, "OWNER") == 0) {
 			valid = 1;
-			if (rules->resolve_names && (!strchr(value, '$') && !strchr(value, '%'))) {
-				char *endptr;
-				strtoul(value, &endptr, 10);
-				if (endptr[0] != '\0') {
-					char owner[32];
-					uid_t uid = lookup_user(value);
-					dbg("replacing username='%s' by id=%i", value, uid);
-					sprintf(owner, "%u", (unsigned int) uid);
-					add_rule_key(rule, &rule->owner, operation, owner);
-					continue;
-				}
-			}
-
 			add_rule_key(rule, &rule->owner, operation, value);
 			continue;
 		}
 
 		if (strcasecmp(key, "GROUP") == 0) {
 			valid = 1;
-			if (rules->resolve_names && (!strchr(value, '$') && !strchr(value, '%'))) {
-				char *endptr;
-				strtoul(value, &endptr, 10);
-				if (endptr[0] != '\0') {
-					char group[32];
-					gid_t gid = lookup_group(value);
-					dbg("replacing groupname='%s' by id=%i", value, gid);
-					sprintf(group, "%u", (unsigned int) gid);
-					add_rule_key(rule, &rule->group, operation, group);
-					continue;
-				}
-			}
-
 			add_rule_key(rule, &rule->group, operation, value);
 			continue;
 		}
@@ -637,13 +611,12 @@
 	return retval;
 }
 
-int udev_rules_init(struct udev_rules *rules, int resolve_names)
+int udev_rules_init(struct udev_rules *rules)
 {
 	struct stat stats;
 	int retval;
 
 	memset(rules, 0x00, sizeof(struct udev_rules));
-	rules->resolve_names = resolve_names;
 
 	/* parse rules file or all matching files in directory */
 	if (stat(udev_rules_filename, &stats) != 0)
diff -u udev-096.orig/udevstart.c udev-096/udevstart.c
--- udev-096.orig/udevstart.c	2006-08-05 00:16:20.000000000 +0100
+++ udev-096/udevstart.c	2006-08-05 00:17:11.000000000 +0100
@@ -361,7 +361,7 @@
 	alarm(UDEV_ALARM_TIMEOUT);
 
 	sysfs_init();
-	udev_rules_init(&rules, 1);
+	udev_rules_init(&rules);
 
 	udev_scan_class(&device_list);
 	udev_scan_block(&device_list);
diff -u udev-096.orig/udevtest.c udev-096/udevtest.c
--- udev-096.orig/udevtest.c	2006-08-05 00:16:20.000000000 +0100
+++ udev-096/udevtest.c	2006-08-05 00:17:11.000000000 +0100
@@ -83,7 +83,7 @@
 			devpath = argv[1];
 
 	sysfs_init();
-	udev_rules_init(&rules, 0);
+	udev_rules_init(&rules);
 
 	dev = sysfs_device_get(devpath);
 	if (dev == NULL) {

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

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

[-- Attachment #4: Type: text/plain, Size: 226 bytes --]

_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [PATCH] Only lookup uid/gid when applying rules

[Kay Sievers]

On Sat, 2006-08-05 at 00:32 +0100, Roy Marples wrote:
> On Tuesday 01 August 2006 13:35, you wrote:
> > On Tue, 2006-08-01 at 11:35 +0100, Roy Marples wrote:
> > > Attached is a patch that stops udev from doing a uid/gid lookup unless it
> > > is actually going to use the rule.
> >
> > Nope, that doesn't really help, we want to to resolve the names once on
> > udevd startup and not from every event process again and again.
> 
> OK, this patch again removes the uid/gid lookup when loading rules as it's the 
> root cause of the problem. I also don't see a flag being of any benefit.

Well, the real problem is that we block n=(number of GROUP+OWNER keys)
ldap timeouts in glibc waiting for a network connection. This patch just
moves the problem away from the daemon startup to all event processes
which use OWNER/GROUP.
Right, it may be faster to wait for n timeouts with n event processes in
parallel, than to wait for n timeouts in a row with the daemon. But I
don't think, this fixes the problem proper.

> However, this patch stores the looked up uid/gid back in the rule for future 
> event processes on the same rule. So, this should also make us more efficient 
> by only looking up the uid/gid for each rule when it's need, but only once 
> per rule.

I don't see where you write to the rules. And even when, you can't reach
the rules from an event process. Your accessible memory image is just a
clone() of the daemon, which will go away with exit(), so you will never
store anything for other events from there.

Currently - with or without this patch - on a LDAP configuration that
does not return anything on bootup, we assign all OWNER/GROUP
permissions to "root". This can't be fixed later in the boot cycle, so
we have a serious problem anyway. The question is, if we really should
assign it to "root", or maybe fall-back to the built-in /etc/{passwd,groups}
parser? 

Kay


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CIDÞVDEV
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [PATCH] Only lookup uid/gid when applying rules

[Marco d'Itri]

On Aug 05, Kay Sievers <kay.sievers@vrfy.org> wrote:

> Currently - with or without this patch - on a LDAP configuration that
> does not return anything on bootup, we assign all OWNER/GROUP
> permissions to "root". This can't be fixed later in the boot cycle, so
> we have a serious problem anyway. The question is, if we really should
> assign it to "root", or maybe fall-back to the built-in /etc/{passwd,groups}
> parser? 
I believe that falling back to the local static database would be more
confusing. And probably useless too, since on a properly configured
system an user would be first looked up in /etc/passwd and only if not
there LDAP would be used.
Debian had similar problems, and after quickly reading his introduction
I did not find anything questionable in Roy's patch.

-- 
ciao,
Marco

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CIDÞVDEV
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [PATCH] Only lookup uid/gid when applying rules

[Kay Sievers]

On Sat, 2006-08-05 at 02:29 +0200, Marco d'Itri wrote:
> On Aug 05, Kay Sievers <kay.sievers@vrfy.org> wrote:
> 
> > Currently - with or without this patch - on a LDAP configuration that
> > does not return anything on bootup, we assign all OWNER/GROUP
> > permissions to "root". This can't be fixed later in the boot cycle, so
> > we have a serious problem anyway. The question is, if we really should
> > assign it to "root", or maybe fall-back to the built-in /etc/{passwd,groups}
> > parser? 
> I believe that falling back to the local static database would be more
> confusing. And probably useless too, since on a properly configured
> system an user would be first looked up in /etc/passwd and only if not
> there LDAP would be used.
> Debian had similar problems, and after quickly reading his introduction
> I did not find anything questionable in Roy's patch.

Well, on bootup we trigger events for _all_ devices, which will use most
of the rules, and we require the uid/gid to be resolved at that point in
time, long before we have access to the network. That's the issue we
need to solve, not to make it only faster to fail and assign "root" to
all nodes, be it the daemon or the event process who's doing that.

Kay


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CIDÞVDEV
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [PATCH] Only lookup uid/gid when applying rules

[Marco d'Itri]

On Aug 05, Kay Sievers <kay.sievers@vrfy.org> wrote:

> Well, on bootup we trigger events for _all_ devices, which will use most
> of the rules, and we require the uid/gid to be resolved at that point in
> time, long before we have access to the network. That's the issue we
> need to solve, not to make it only faster to fail and assign "root" to
> all nodes, be it the daemon or the event process who's doing that.
Documenting that this is a configuration error and will not work is
still better than the current situation of udev hanging at boot time.

-- 
ciao,
Marco

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CIDÞVDEV
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [PATCH] Only lookup uid/gid when applying rules

[Kay Sievers]

On Sat, 2006-08-05 at 02:43 +0200, Marco d'Itri wrote:
> On Aug 05, Kay Sievers <kay.sievers@vrfy.org> wrote:
> 
> > Well, on bootup we trigger events for _all_ devices, which will use most
> > of the rules, and we require the uid/gid to be resolved at that point in
> > time, long before we have access to the network. That's the issue we
> > need to solve, not to make it only faster to fail and assign "root" to
> > all nodes, be it the daemon or the event process who's doing that.
> Documenting that this is a configuration error and will not work is
> still better than the current situation of udev hanging at boot time.

Right, I completely agree. It is kind of a bug, to wait n times for the
glibc timeout on udevd startup. That should be changed and udevd should
probably skip further attempts after the first lookup timeout. But what
would be the proper behavior? Assign "root", if we can't resolve? Look
in /etc?

Kay


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CIDÞVDEV
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [PATCH] Only lookup uid/gid when applying rules

[Marco d'Itri]

On Aug 05, Kay Sievers <kay.sievers@vrfy.org> wrote:

> Right, I completely agree. It is kind of a bug, to wait n times for the
> glibc timeout on udevd startup. That should be changed and udevd should
> probably skip further attempts after the first lookup timeout. But what
> would be the proper behavior? Assign "root", if we can't resolve? Look
> in /etc?
The static database is supposed to be used by the libc before attemping
to use LDAP, so trying again would not be useful.

-- 
ciao,
Marco

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CIDÞVDEV
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [PATCH] Only lookup uid/gid when applying rules

[Roy Marples]

On Saturday 05 August 2006 03:05, Kay Sievers wrote:
> On Sat, 2006-08-05 at 02:49 +0100, Roy Marples wrote:
> > On Saturday 05 August 2006 02:35, you wrote:
> > > Resolving in the event process will work around the current problem,
> > > that we fail, even if we don't have a matching device, that's true, but
> > > the real problem that we block still persists. Sure, it will solve
> > > Gentoo's "tss" problem, but that's not enough reason to add resolving
> > > of id's to every event process.
> >
> > So tell me how to fix this in udev.
>
> We may be able to handle a lookup timeout. You could set a reasonable
> bind_timelimit in ldap.conf, so it will not block for more than a few
> seconds.

Ok lets say that udev ships with a new rule that has a new user/group lookup 
called "foo".

You have instantly broken existing LDAP installs that do not have foo 
in /etc/passwd or /etc/group

Now should the onus be on distro's to magically add foo even though it *may* 
be not needed, or should it be on udev to work out if it even needs to lookup 
the user/group "foo"?

-- 
Roy Marples <uberlord@gentoo.org>
Gentoo/Linux Developer (baselayout, networking)

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CIDÞVDEV
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [PATCH] Only lookup uid/gid when applying rules

[Kay Sievers]

On Sat, 2006-08-05 at 03:20 +0100, Roy Marples wrote:
> On Saturday 05 August 2006 03:05, Kay Sievers wrote:
> > On Sat, 2006-08-05 at 02:49 +0100, Roy Marples wrote:
> > > On Saturday 05 August 2006 02:35, you wrote:
> > > > Resolving in the event process will work around the current problem,
> > > > that we fail, even if we don't have a matching device, that's true, but
> > > > the real problem that we block still persists. Sure, it will solve
> > > > Gentoo's "tss" problem, but that's not enough reason to add resolving
> > > > of id's to every event process.
> > >
> > > So tell me how to fix this in udev.
> >
> > We may be able to handle a lookup timeout. You could set a reasonable
> > bind_timelimit in ldap.conf, so it will not block for more than a few
> > seconds.
> 
> Ok lets say that udev ships with a new rule that has a new user/group lookup 
> called "foo".

Udev usually ships no rules with id's, which are not part of the base
system.

> You have instantly broken existing LDAP installs that do not have foo 
> in /etc/passwd or /etc/group

Right, then you system is broken and it will result in a bootup delay,
which is acceptable.

> Now should the onus be on distro's to magically add foo even though it *may* 
> be not needed, or should it be on udev to work out if it even needs to lookup 
> the user/group "foo"?

You should fix your base-system, which is causing the trouble, simple as
that. 

And what's the problem? With that one unresolvable group name, it will
take 60 seconds to timeout on bootup waiting for ldap. Then continue,
and you can fix that problem on your system:
  time /sbin/udevd --daemon
  real    1m0.013s

If you really care about such a case, put:
  bind_timelimit 3
in your:
  /etc/ldap.conf
and it will take 6 seconds:
  time /sbin/udevd --daemon
  real    0m6.012s

You are trying to trade an valuable optimization of udev for a rare-case
configuration error that happened in your distro, cause of bad
packaging. That's really not an interesting deal.

Again, we could try to work around such an error, that would be nice to
have, but not until we are sure what's the right fix for it, and pushing
the resolving into the event processes isn't it.

Kay


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CIDÞVDEV
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [PATCH] Only lookup uid/gid when applying rules

[Roy Marples]

On Saturday 05 August 2006 03:42, you wrote:
> > Now should the onus be on distro's to magically add foo even though it
> > *may* be not needed, or should it be on udev to work out if it even needs
> > to lookup the user/group "foo"?
>
> You should fix your base-system, which is causing the trouble, simple as
> that.

So, we have to keep in sync all user/groups udev requires?

As opposed to adding them dynamically when the user actually installs the 
software the uses said users/groups? Or even has the hardware?

Case in point - my LDAP server has no need for the user/group tss other than 
the fact that udev requires it due to a rule that will never be applied.

And you still think my base-system should be fixed? What  happens down the 
road when you next require user foo and group bar due to a rule which may or 
may not be needed?

I give up. It's hopeless. I just cannot convice you that it's better to be 
slow than broken.

-- 
Roy Marples <uberlord@gentoo.org>
Gentoo/Linux Developer (baselayout, networking)

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CIDÞVDEV
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [PATCH] Only lookup uid/gid when applying rules

[Kay Sievers]

On Sat, 2006-08-05 at 03:59 +0100, Roy Marples wrote:
> On Saturday 05 August 2006 03:42, you wrote:
> > > Now should the onus be on distro's to magically add foo even though it
> > > *may* be not needed, or should it be on udev to work out if it even needs
> > > to lookup the user/group "foo"?
> >
> > You should fix your base-system, which is causing the trouble, simple as
> > that.
> 
> So, we have to keep in sync all user/groups udev requires?

Yes, and not only for udev.

> As opposed to adding them dynamically when the user actually installs the 
> software the uses said users/groups? Or even has the hardware?

Well, just make sure you don't have the rules that require entries you
don't want to add to your system.

> Case in point - my LDAP server has no need for the user/group tss other than 
> the fact that udev requires it due to a rule that will never be applied.

Sure, put it in /etc, where system groups belong.

> And you still think my base-system should be fixed?

Definitely.

> What happens down the road when you next require user foo and group
> bar due to a rule which may or may not be needed?

Just make sure, the udev rules are in sync with the rest of your system.

> I give up. It's hopeless. I just cannot convice you that it's better to be 
> slow than broken.

Right, you can't. Not every system is broken like yours seems to be,
therefore we will not optimize udev for such broken systems.

Kay


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CIDÞVDEV
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [PATCH] Only lookup uid/gid when applying rules

[Roy Marples]

On Saturday 05 August 2006 04:07, Kay Sievers wrote:
> > Case in point - my LDAP server has no need for the user/group tss other
> > than the fact that udev requires it due to a rule that will never be
> > applied.
>
> Sure, put it in /etc, where system groups belong.

See, I don't see that as a system group. My systems, and many other Gentoo 
systems have done with the tss user/group for a while now.

So you're deciding whats a system user/group? Fine with me, I'm washing my 
hands of this. Heck, I have one LDAP system at work where I've made a fairly 
trivial patch to fix this issue and future "hey guys! new devices, new rules! 
oh yeah, LDAP users have to have em in /etc/passwd AND do files ldap instead 
of ldap files"

>
> > And you still think my base-system should be fixed?
>
> Definitely.
>
> > What happens down the road when you next require user foo and group
> > bar due to a rule which may or may not be needed?
>
> Just make sure, the udev rules are in sync with the rest of your system.
>
> > I give up. It's hopeless. I just cannot convice you that it's better to
> > be slow than broken.
>
> Right, you can't. Not every system is broken like yours seems to be,
> therefore we will not optimize udev for such broken systems.

See, the systems aren't broken at all.

The only brokenness is udevs blind assumption that every user/group in every 
rule must be available at boot time.

Remember, udev loads all rules in /etc/udev/rules.d
So you're saying that all user groups present in /etc/udev/rules.d must be in 
base-system? Does that mean you also dictate to all providers 
for /etc/udev/rules.d what user/groups they are allowed or is it a general 
free for all in that base-system must have mappings for all potential 
user/groups?

-- 
Roy Marples <uberlord@gentoo.org>
Gentoo/Linux Developer (baselayout, networking)

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CIDÞVDEV
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel