linux-kernel - Re: RFC: UIO null parent for __uio_register_device and uio_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]
Date:	Wed, 19 Jan 2011 07:42:12 -0800
From:	Earl Chew <echew@...acom.com>
To:	"Hans J. Koch" <hjk@...sjkoch.de>
CC:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Hans J. Koch" <hjk@...utronix.de>,
	Greg Kroah-Hartman <greg@...ah.com>
Subject: Re: RFC: UIO  null parent for __uio_register_device and uio_device_name()

Hans,

Thanks for the quick response.

>> o Allow a null parent during uio_register_device. We've had
>>   situations where there was no convenient parent. We could
>>   concoct a parent, but it seemed to make sense to allow
>>   for a null.
> 
> Hmm. I saw many UIO drivers over the years but we never had that
> problem. In the probe() function of your UIO driver you get a
> suitable device as parameter. Why don't you just use it?

We build custom hardware, and UIO has served us well by providing
a way out of kernel space.

[ I'm calling the driver we write the "UIO client driver" to
  avoid confusion with the Linux UIO driver. ]

In some implementations, the HW binds directly to the CPU
physical address space. In particular, it is not accessed via
PCI space. Thus there is no probe() function required at the
UIO client driver.

The physical address assignments for the device are fixed
at system design time, and the UIO client driver (for the custom hardware)
has those addresses built into it.

No probing is required in this scenario.

> Yes, we intentionally defined struct uio_device in uio.c and not in
> a header file to prevent driver authors from using it ;-)

Yes. This is a good pattern. I like using it too  ;-)

>> +const char *uio_device_name(struct uio_info *info)
>> +{
>> +       return dev_name(info->uio_dev->dev);
> 
> No NULL checks? Note that info->uio_dev can be NULL if uio_register_device()
> failed...

Apologies. I've tightened up the code.  See below.

> The function above returns strings like "uio0", "uio1", and so on. What's
> the value of that? To identify a certain driver or distinguish several
> instances of it, the "name" member of struct uio_info should be used.
> 
> But maybe I just miss the point. What's your use case?

This is how our code currently works.

1. UIO client driver registers with Linux UIO driver.
2. UIO client driver advertises the name of the Linux UIO driver
   instance to which it is bound (eg "uio2") to our custom
   application
3. Our custom application queries for the name of the driver
   to use (eg "uio2")
4. Our custom application issues open("/dev/uio2") to access hardware.

Working on the implementation of our new system last night I realise
that our new configuration runs very lean, and I now do not have the
use of udev to populate /dev/uio[0-9].

This new configuration runs so lean that I don't have access to sysfs.
I've asked about including sysfs, but there are good reasons not to
include it. This means that /sys/class/uio[0-9]/... will not be available.

The result is that I need to figure out the major/minor device number in
order to access the Linux UIO device. I will advertise this through /proc/
entries specific to the UIO client driver. We can then use this information
to mknod /dev/uio[0-9].

I've added uio_device_chrdev() alongside uio_device_name() to allow
us to figure out the coordinates of the Linux UIO device.

Earl



--- /tmp/uio_driver.h.orig      2011-01-18 19:42:13.777452360 -0800
+++ /tmp/uio_driver.h   2011-01-19 07:26:58.037452472 -0800
@@ -105,6 +105,8 @@
 }
 extern void uio_unregister_device(struct uio_info *info);
 extern void uio_event_notify(struct uio_info *info);
+extern const char *uio_device_name(struct uio_device *info);
+extern dev_t uio_device_chrdev(struct uio_device *info);

 /* defines for uio_info->irq */
 #define UIO_IRQ_CUSTOM -1


--- /tmp/uio.c.orig     2011-01-18 17:38:17.157452875 -0800
+++ /tmp/uio.c  2011-01-19 07:39:48.457461684 -0800
@@ -808,6 +808,36 @@
 }

 /**
+ * uio_device_chrdev - obtain the name of the registered device
+ * @info:      UIO device capabilities
+ *
+ * returns device number or NULL if not available.
+ */
+dev_t uio_device_chrdev(struct uio_info *info)
+{
+       if (info && info->uio_dev)
+               return MKDEV(uio_major, info->uio_dev->minor);
+
+       return 0;
+}
+EXPORT_SYMBOL_GPL(uio_device_chrdev);
+
+/**
+ * uio_device_name - obtain the name of the registered device
+ * @info:      UIO device capabilities
+ *
+ * returns name of device or NULL if not available.
+ */
+const char *uio_device_name(struct uio_info *info)
+{
+       if (info && info->uio_dev && info->uio_dev->dev)
+               return dev_name(info->uio_dev->dev);
+
+       return NULL;
+}
+EXPORT_SYMBOL_GPL(uio_device_name);
+
+/**
  * uio_register_device - register a new userspace IO device
  * @owner:     module that creates the new device
  * @parent:    parent device
@@ -822,7 +852,7 @@
        struct uio_device *idev;
        int ret = 0;

-       if (!parent || !info || !info->name || !info->version)
+       if (!info || !info->name || !info->version)
                return -EINVAL;

        info->uio_dev = NULL;

--
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/