[PATCH, UDEV] fix permission inconsistency in relation to symlinks.

[PATCH, UDEV] fix permission inconsistency in relation to symlinks.

[Michael Buesch]

[-- Attachment #1.1: Type: text/plain, Size: 1906 bytes --]

Hi,

This fixes a rules.d permission apply inconsistency.
Here's an example of what happens without the patch:


First case without the patch:

KERNEL="hd*", GROUP="disk", MODE="0660"
KERNEL="hdc", SYMLINK="cdrecorder", MODE="0666"

mb@lfs:~$ ls -la /dev/hdc 
brw-rw----  1 root disk 22, 0 Feb  5 14:08 /dev/hdc



Second case without the patch:

KERNEL="hd*", GROUP="disk", MODE="0660"
KERNEL="hdc", MODE="0666"

mb@lfs:~$ ls -la /dev/hdc 
brw-rw-rw-  1 root disk 22, 0 Feb  5 14:08 /dev/hdc



With the patch applied:

KERNEL="hd*", GROUP="disk", MODE="0660"
KERNEL="hdc", SYMLINK="cdrecorder", MODE="0666"

mb@lfs:~$ ls -la /dev/hdc 
brw-rw-rw-  1 root disk 22, 0 Feb  5 14:08 /dev/hdc


-- 
Regards Michael Buesch  [ http://www.tuxsoft.de.vu ]


===== namedev.c 1.173 vs edited =====
--- 1.173/namedev.c	2004-12-21 17:41:02 +01:00
+++ edited/namedev.c	2005-02-05 14:04:44 +01:00
@@ -716,14 +716,16 @@
 	list_for_each_entry(dev, &config_device_list, node) {
 		dbg("process rule");
 		if (match_rule(udev, dev, class_dev, sysfs_device) == 0) {
-			if (dev->name[0] == '\0' && dev->symlink[0] == '\0') {
+
+			if (dev->name[0] == '\0' && dev->symlink[0] == '\0' &&
+			    dev->mode == 0000 && dev->owner[0] == '\0' && dev->group[0] == '\0') {
 				/* empty name, symlink, perms will not create any node */
-				if (dev->mode == 0000 && dev->owner[0] == '\0' && dev->group[0] == '\0') {
-					info("configured rule in '%s[%i]' applied, '%s' is ignored",
-					     dev->config_file, dev->config_line, udev->kernel_name);
-					return -1;
-				}
+				info("configured rule in '%s[%i]' applied, '%s' is ignored",
+				     dev->config_file, dev->config_line, udev->kernel_name);
+				return -1;
+			}
 
+			if (dev->name[0] == '\0') {
 				/* apply permissions only rule values */
 				if (dev->mode != 0000) {
 					udev->mode = dev->mode;

[-- Attachment #1.2: udev_fix_symlink_and_perms.diff --]
[-- Type: text/x-diff, Size: 1117 bytes --]

===== namedev.c 1.173 vs edited =====
--- 1.173/namedev.c	2004-12-21 17:41:02 +01:00
+++ edited/namedev.c	2005-02-05 14:04:44 +01:00
@@ -716,14 +716,16 @@
 	list_for_each_entry(dev, &config_device_list, node) {
 		dbg("process rule");
 		if (match_rule(udev, dev, class_dev, sysfs_device) == 0) {
-			if (dev->name[0] == '\0' && dev->symlink[0] == '\0') {
+
+			if (dev->name[0] == '\0' && dev->symlink[0] == '\0' &&
+			    dev->mode == 0000 && dev->owner[0] == '\0' && dev->group[0] == '\0') {
 				/* empty name, symlink, perms will not create any node */
-				if (dev->mode == 0000 && dev->owner[0] == '\0' && dev->group[0] == '\0') {
-					info("configured rule in '%s[%i]' applied, '%s' is ignored",
-					     dev->config_file, dev->config_line, udev->kernel_name);
-					return -1;
-				}
+				info("configured rule in '%s[%i]' applied, '%s' is ignored",
+				     dev->config_file, dev->config_line, udev->kernel_name);
+				return -1;
+			}
 
+			if (dev->name[0] == '\0') {
 				/* apply permissions only rule values */
 				if (dev->mode != 0000) {
 					udev->mode = dev->mode;

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH, UDEV] fix permission inconsistency in relation to

[Kay Sievers]

On Sat, 2005-02-05 at 14:26 +0100, Michael Buesch wrote:
> This fixes a rules.d permission apply inconsistency.
> Here's an example of what happens without the patch:

> First case without the patch:
> 
> KERNEL="hd*", GROUP="disk", MODE="0660"
> KERNEL="hdc", SYMLINK="cdrecorder", MODE="0666"
> brw-rw----  1 root disk 22, 0 Feb  5 14:08 /dev/hdc

> Second case without the patch:
> 
> KERNEL="hd*", GROUP="disk", MODE="0660"
> KERNEL="hdc", MODE="0666"
> brw-rw-rw-  1 root disk 22, 0 Feb  5 14:08 /dev/hdc

Both cases are correct behavior in the current model, where symlink-only
rules can not set the permissions of a node. It was the same with the
"old" .permissions files.

Should we change that now, so that permissions are applied from a
symlink-only rule too?

Kay



-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl
_______________________________________________
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, UDEV] fix permission inconsistency in relation to symlinks.

[Marco d'Itri]

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

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

> Should we change that now, so that permissions are applied from a
> symlink-only rule too?
Yes!
I think it's the sensible semantic: the second rule adds a new SYMLINK
*and* sets the permissions of the node, which are two distinct
operations applied by the same rule.
Unless you think it's broken, I will add the patch to the debian package
of 051 (almost ready).

-- 
ciao,
Marco

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH, UDEV] fix permission inconsistency in relation to symlinks.

[Michael Buesch]

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

Quoting Kay Sievers <kay.sievers@vrfy.org>:
> On Sat, 2005-02-05 at 14:26 +0100, Michael Buesch wrote:
> > This fixes a rules.d permission apply inconsistency.
> > Here's an example of what happens without the patch:
> 
> > First case without the patch:
> > 
> > KERNEL="hd*", GROUP="disk", MODE="0660"
> > KERNEL="hdc", SYMLINK="cdrecorder", MODE="0666"
> > brw-rw----  1 root disk 22, 0 Feb  5 14:08 /dev/hdc
> 
> > Second case without the patch:
> > 
> > KERNEL="hd*", GROUP="disk", MODE="0660"
> > KERNEL="hdc", MODE="0666"
> > brw-rw-rw-  1 root disk 22, 0 Feb  5 14:08 /dev/hdc
> 
> Both cases are correct behavior in the current model, where symlink-only
> rules can not set the permissions of a node. It was the same with the
> "old" .permissions files.
> 
> Should we change that now, so that permissions are applied from a
> symlink-only rule too?

I was _very_ confused when I saw this behaviour. It took
me a lot of time to figure out what's really going on there and
how to workaround it.

> Kay
> 
> 
> 

-- 
Regards Michael Buesch  [ http://www.tuxsoft.de.vu ]



[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH, UDEV] fix permission inconsistency in relation to

[Kay Sievers]

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

On Sat, 2005-02-05 at 15:33 +0100, Marco d'Itri wrote:
> On Feb 05, Kay Sievers <kay.sievers@vrfy.org> wrote:
> 
> > Should we change that now, so that permissions are applied from a
> > symlink-only rule too?
> Yes!
> I think it's the sensible semantic: the second rule adds a new SYMLINK
> *and* sets the permissions of the node, which are two distinct
> operations applied by the same rule.

Yeah, we should do that now.

> Unless you think it's broken, I will add the patch to the debian package
> of 051 (almost ready).

I think we can make it even simpler, by applying the permission
unconditionally, right?

Thanks,
Kay

[-- Attachment #2: udev-symlink-perms-01.patch --]
[-- Type: text/x-patch, Size: 3636 bytes --]

===== namedev.c 1.173 vs edited =====
--- 1.173/namedev.c	2004-12-21 17:41:02 +01:00
+++ edited/namedev.c	2005-02-05 15:51:06 +01:00
@@ -670,7 +670,6 @@ try_parent:
 		dbg("sysfs_device->path='%s'", sysfs_device->path);
 		dbg("sysfs_device->bus_id='%s'", sysfs_device->bus_id);
 	}
-
 }
 
 int namedev_name_device(struct udevice *udev, struct sysfs_class_device *class_dev)
@@ -716,32 +715,32 @@ int namedev_name_device(struct udevice *
 	list_for_each_entry(dev, &config_device_list, node) {
 		dbg("process rule");
 		if (match_rule(udev, dev, class_dev, sysfs_device) == 0) {
-			if (dev->name[0] == '\0' && dev->symlink[0] == '\0') {
-				/* empty name, symlink, perms will not create any node */
-				if (dev->mode == 0000 && dev->owner[0] == '\0' && dev->group[0] == '\0') {
-					info("configured rule in '%s[%i]' applied, '%s' is ignored",
-					     dev->config_file, dev->config_line, udev->kernel_name);
-					return -1;
-				}
-
-				/* apply permissions only rule values */
-				if (dev->mode != 0000) {
-					udev->mode = dev->mode;
-					dbg("applied mode=%#o to '%s'", udev->mode, udev->kernel_name);
-				}
-				if (dev->owner[0] != '\0') {
-					strfieldcpy(udev->owner, dev->owner);
-					apply_format(udev, udev->owner, sizeof(udev->owner), class_dev, sysfs_device);
-					dbg("applied owner='%s' to '%s'", udev->owner, udev->kernel_name);
-				}
-				if (dev->group[0] != '\0') {
-					strfieldcpy(udev->group, dev->group);
-					apply_format(udev, udev->group, sizeof(udev->group), class_dev, sysfs_device);
-					dbg("applied group='%s' to '%s'", udev->group, udev->kernel_name);
-				}
+
+			/* empty name, symlink, perms will not create any node */
+			if (dev->name[0] == '\0' && dev->symlink[0] == '\0' &&
+			    dev->mode == 0000 && dev->owner[0] == '\0' && dev->group[0] == '\0') {
+				info("configured rule in '%s[%i]' applied, '%s' is ignored",
+				     dev->config_file, dev->config_line, udev->kernel_name);
+				return -1;
+			}
+
+			/* apply permissions */
+			if (dev->mode != 0000) {
+				udev->mode = dev->mode;
+				dbg("applied mode=%#o to '%s'", udev->mode, udev->kernel_name);
+			}
+			if (dev->owner[0] != '\0') {
+				strfieldcpy(udev->owner, dev->owner);
+				apply_format(udev, udev->owner, sizeof(udev->owner), class_dev, sysfs_device);
+				dbg("applied owner='%s' to '%s'", udev->owner, udev->kernel_name);
+			}
+			if (dev->group[0] != '\0') {
+				strfieldcpy(udev->group, dev->group);
+				apply_format(udev, udev->group, sizeof(udev->group), class_dev, sysfs_device);
+				dbg("applied group='%s' to '%s'", udev->group, udev->kernel_name);
 			}
 
-			/* collect symlinks for the final matching rule */
+			/* collect symlinks for this or the final matching rule */
 			if (dev->symlink[0] != '\0') {
 				char temp[NAME_SIZE];
 
@@ -774,19 +773,9 @@ int namedev_name_device(struct udevice *
 					goto exit;
 
 				udev->partitions = dev->partitions;
-				if (dev->mode != 0000)
-					udev->mode = dev->mode;
-				if (dev->owner[0] != '\0') {
-					strfieldcpy(udev->owner, dev->owner);
-					apply_format(udev, udev->owner, sizeof(udev->owner), class_dev, sysfs_device);
-				}
-				if (dev->group[0] != '\0') {
-					strfieldcpy(udev->group, dev->group);
-					apply_format(udev, udev->group, sizeof(udev->group), class_dev, sysfs_device);
-				}
 
-				dbg("name, '%s' is going to have owner='%s', group='%s', mode = %#o",
-				    udev->name, udev->owner, udev->group, udev->mode);
+				dbg("name, '%s' is going to have owner='%s', group='%s', mode=%#o partitions=%i",
+				    udev->name, udev->owner, udev->group, udev->mode, udev->partitions);
 
 				goto exit;
 			}

Re: [PATCH, UDEV] fix permission inconsistency in relation to symlinks.

[Michael Buesch]

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

Quoting Kay Sievers <kay.sievers@vrfy.org>:
> On Sat, 2005-02-05 at 15:33 +0100, Marco d'Itri wrote:
> > On Feb 05, Kay Sievers <kay.sievers@vrfy.org> wrote:
> > 
> > > Should we change that now, so that permissions are applied from a
> > > symlink-only rule too?
> > Yes!
> > I think it's the sensible semantic: the second rule adds a new SYMLINK
> > *and* sets the permissions of the node, which are two distinct
> > operations applied by the same rule.
> 
> Yeah, we should do that now.
> 
> > Unless you think it's broken, I will add the patch to the debian package
> > of 051 (almost ready).
> 
> I think we can make it even simpler, by applying the permission
> unconditionally, right?

Yes, seems correct.
A first test showed that this works.

PS: Please CC: me on this topic, as I'm not subscribed to the mailing list.
Thanks.

> Thanks,
> Kay
> 

-- 
Regards Michael Buesch  [ http://www.tuxsoft.de.vu ]



[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH, UDEV] fix permission inconsistency in relation to

[Kay Sievers]

On Sat, 2005-02-05 at 16:09 +0100, Michael Buesch wrote:
> Quoting Kay Sievers <kay.sievers@vrfy.org>:
> > On Sat, 2005-02-05 at 15:33 +0100, Marco d'Itri wrote:
> > > On Feb 05, Kay Sievers <kay.sievers@vrfy.org> wrote:
> > > 
> > > > Should we change that now, so that permissions are applied from a
> > > > symlink-only rule too?
> > > Yes!
> > > I think it's the sensible semantic: the second rule adds a new SYMLINK
> > > *and* sets the permissions of the node, which are two distinct
> > > operations applied by the same rule.
> > 
> > Yeah, we should do that now.
> > 
> > > Unless you think it's broken, I will add the patch to the debian package
> > > of 051 (almost ready).
> > 
> > I think we can make it even simpler, by applying the permission
> > unconditionally, right?
> 
> Yes, seems correct.
> A first test showed that this works.

I've applied it to my tree, thanks.

> PS: Please CC: me on this topic, as I'm not subscribed to the mailing list.
> Thanks.

Sorry, you slipped it the To:.
Didn't you get message directly?

Thanks,
Kay



-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl
_______________________________________________
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, UDEV] fix permission inconsistency in relation to symlinks.

[Michael Buesch]

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

Quoting Kay Sievers <kay.sievers@vrfy.org>:
> > PS: Please CC: me on this topic, as I'm not subscribed to the mailing list.
> > Thanks.
> 
> Sorry, you slipped it the To:.
> Didn't you get message directly?

Oh, I got the message. The PS was for Marco d'Itri. ;)
But I'm subscribed now. So the problem does not exist
any longer.

> Thanks,
> Kay

-- 
Regards Michael Buesch  [ http://www.tuxsoft.de.vu ]



[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH, UDEV] fix permission inconsistency in relation to

[Kay Sievers]

On Sat, 2005-02-05 at 16:25 +0100, Michael Buesch wrote:
> Quoting Kay Sievers <kay.sievers@vrfy.org>:
> > > PS: Please CC: me on this topic, as I'm not subscribed to the mailing list.
> > > Thanks.
> > 
> > Sorry, you slipped it the To:.
> > Didn't you get message directly?
> 
> Oh, I got the message. The PS was for Marco d'Itri. ;)
> But I'm subscribed now. So the problem does not exist
> any longer.

Yeah, but with _this_ list, which sometimes takes a couple of hours to
deliver, you still have the same problem... So CC: is better, sure.

Kay



-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl
_______________________________________________
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