linux-kernel - Re: [PATCH mm] sysfs: add /sys/dev/usb to handle CONFIG_USB_DEVICE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1208505291.2514.9.camel@lov.site>
Date:	Fri, 18 Apr 2008 09:54:51 +0200
From:	Kay Sievers <kay.sievers@...y.org>
To:	Dan Williams <dan.j.williams@...el.com>
Cc:	Greg KH <greg@...ah.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	linux-usb@...r.kernel.org
Subject: Re: [PATCH mm] sysfs: add /sys/dev/usb to handle
	CONFIG_USB_DEVICE_CLASS=y

On Thu, 2008-04-17 at 21:59 -0700, Dan Williams wrote:
> On Thu, Apr 17, 2008 at 8:21 PM, Greg KH <greg@...ah.com> wrote:
> > On Thu, Apr 17, 2008 at 07:11:03PM -0700, Dan Williams wrote:
> >
> > > The deprecated config option CONFIG_USB_DEVICE_CLASS causes class devices
> >  > with duplicate major:minor numbers to be registered.  In effect they
> >  > represent a usb specific address space for major:minor numbers so add 'usb'
> >  > as a directory along side 'block' and 'char'.
> >
> >  Hm, no they do not, they are not a new address space, we are just
> >  reusing them as the other user wasn't using them, and the code is
> >  deprecated and will be removed eventually.  Neither of these char
> >  devices are really hooked up to anything within the kernel, so it
> >  doesn't matter yet.
> >
> >  I wonder how many other duplicates we have floating around...
> >
> 
> Hm, so how about making this an opt-in capability of the class?  At
> device_add() time the class is queried to see if a link should be
> created in /sys/dev/block or /sys/dev/char?

This could work, yes. We could just set a flag in the class, that
prevents the device entries in /sys/dev/.

> Although, this leads to
> inconsistent coverage.  But maybe that does not matter as many of
> these character devices do not have interesting attributes to be
> accessed?

The usual case is that one of the duplicated /sys devices is deprecated,
so it should be fine, to always point to the new one.

> Sigh, I am beginning to wonder if the character device side
> of this capability is a solution looking for a problem...

We will find a solution. :)

The following should work for the common case.

Best,
Kay


From: Kay Sievers <kay.sievers@...y.org>
Subject: sysfs: fix duplicated device number registration in /sys/dev/

If the parent device has the same dev_t, we skip the registration
for the device number at /sys/dev.

Cc: Dan Williams <dan.j.williams@...el.com>
Cc: Greg KH <greg@...ah.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>
Signed-off-by: Kay Sievers <kay.sievers@...y.org>
---

 core.c |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index de925f8..afedadb 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -821,10 +821,13 @@ int device_add(struct device *dev)
 		if (error)
 			goto ueventattrError;
 
-		format_dev_t(devt_str, dev->devt);
-		error = sysfs_create_link(kobj, &dev->kobj, devt_str);
-		if (error)
-			goto devtattrError;
+		/* do not create /sys/dev/ entry if parent already did */
+		if (!(dev->parent && dev->parent->devt == dev->devt)) {
+			format_dev_t(devt_str, dev->devt);
+			error = sysfs_create_link(kobj, &dev->kobj, devt_str);
+			if (error)
+				goto devtattrError;
+		}
 	}
 
 	error = device_add_class_symlinks(dev);
@@ -950,8 +953,10 @@ void device_del(struct device *dev)
 	if (parent)
 		klist_del(&dev->knode_parent);
 	if (MAJOR(dev->devt)) {
-		format_dev_t(devt_str, dev->devt);
-		sysfs_remove_link(device_to_dev_kobj(dev), devt_str);
+		if (!(parent && parent->devt == dev->devt)) {
+			format_dev_t(devt_str, dev->devt);
+			sysfs_remove_link(device_to_dev_kobj(dev), devt_str);
+		}
 		device_remove_file(dev, &devt_attr);
 	}
 	if (dev->class) {


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/