2008-02-13 17:04:20

by Hans J. Koch

[permalink] [raw]
Subject: UIO sysfs attributes broken in 2.6.25-rc1

In 2.6.25-rc1, there's a strange effect:
Although the map attributes "addr" and "size" show up correctly:

-r--r--r-- 1 root root 4096 2008-02-13 17:49 addr
-r--r--r-- 1 root root 4096 2008-02-13 17:49 size

you cannot access them, not even as root. Something like "cat addr"
fails with a no-permission-error. It works fine in 2.6.24-rc8.

This is probably related to this patch:

http://lkml.org/lkml/2008/1/25/149

Unfortunately, I signed-off that one, too... I checked if all sysfs
files appear, but didn't try to access them. Sorry.

Greg, any ideas?

Thanks,
Hans


2008-02-13 17:39:01

by Greg KH

[permalink] [raw]
Subject: Re: UIO sysfs attributes broken in 2.6.25-rc1

On Wed, Feb 13, 2008 at 06:03:04PM +0100, Hans-J??rgen Koch wrote:
> In 2.6.25-rc1, there's a strange effect:
> Although the map attributes "addr" and "size" show up correctly:
>
> -r--r--r-- 1 root root 4096 2008-02-13 17:49 addr
> -r--r--r-- 1 root root 4096 2008-02-13 17:49 size
>
> you cannot access them, not even as root. Something like "cat addr"
> fails with a no-permission-error. It works fine in 2.6.24-rc8.
>
> This is probably related to this patch:
>
> http://lkml.org/lkml/2008/1/25/149
>
> Unfortunately, I signed-off that one, too... I checked if all sysfs
> files appear, but didn't try to access them. Sorry.

Heh, well at least it wasn't just me that thought the patch was correct
:)

> Greg, any ideas?

I'll look into it this afternoon...

thanks,

greg k-h

2008-02-19 09:55:32

by Brandon Philips

[permalink] [raw]
Subject: [PATCH] UIO: introduce sysfs_ops for map_attr_ktype

This fixes two bugs with UIO that cropped up recently in -rc1

1) WARNING: at fs/sysfs/file.c:334 sysfs_open_file when trying to open
a map addr/size file - complaining about missing sysfs_ops for ktype

2) Permission denied when reading uio/uio0/maps/map0/{addr,size} when
files are mode S_IRUGO

Also fix a typo: attr_attribute -> addr_attribute

Signed-off-by: Brandon Philips <[email protected]>

---
drivers/uio/uio.c | 54 ++++++++++++++++++++++++++++++---------------
1 file changed, 37 insertions(+), 17 deletions(-)

Index: linux-2.6/drivers/uio/uio.c
===================================================================
--- linux-2.6.orig/drivers/uio/uio.c
+++ linux-2.6/drivers/uio/uio.c
@@ -57,29 +57,29 @@ struct uio_map {
};
#define to_map(map) container_of(map, struct uio_map, kobj)

-
-static ssize_t map_attr_show(struct kobject *kobj, struct kobj_attribute *attr,
- char *buf)
+static ssize_t map_addr_show(struct uio_mem *mem, char *buf)
{
- struct uio_map *map = to_map(kobj);
- struct uio_mem *mem = map->mem;
-
- if (strncmp(attr->attr.name, "addr", 4) == 0)
- return sprintf(buf, "0x%lx\n", mem->addr);
-
- if (strncmp(attr->attr.name, "size", 4) == 0)
- return sprintf(buf, "0x%lx\n", mem->size);
+ return sprintf(buf, "0x%lx\n", mem->addr);
+}

- return -ENODEV;
+static ssize_t map_size_show(struct uio_mem *mem, char *buf)
+{
+ return sprintf(buf, "0x%lx\n", mem->size);
}

-static struct kobj_attribute attr_attribute =
- __ATTR(addr, S_IRUGO, map_attr_show, NULL);
-static struct kobj_attribute size_attribute =
- __ATTR(size, S_IRUGO, map_attr_show, NULL);
+struct uio_sysfs_entry {
+ struct attribute attr;
+ ssize_t (*show)(struct uio_mem *, char *);
+ ssize_t (*store)(struct uio_mem *, const char *, size_t);
+};
+
+static struct uio_sysfs_entry addr_attribute =
+ __ATTR(addr, S_IRUGO, map_addr_show, NULL);
+static struct uio_sysfs_entry size_attribute =
+ __ATTR(size, S_IRUGO, map_size_show, NULL);

static struct attribute *attrs[] = {
- &attr_attribute.attr,
+ &addr_attribute.attr,
&size_attribute.attr,
NULL, /* need to NULL terminate the list of attributes */
};
@@ -90,8 +90,28 @@ static void map_release(struct kobject *
kfree(map);
}

+static ssize_t map_type_show(struct kobject *kobj, struct attribute *attr,
+ char *buf)
+{
+ struct uio_map *map = to_map(kobj);
+ struct uio_mem *mem = map->mem;
+ struct uio_sysfs_entry *entry;
+
+ entry = container_of(attr, struct uio_sysfs_entry, attr);
+
+ if (!entry->show)
+ return -EIO;
+
+ return entry->show(mem, buf);
+}
+
+static struct sysfs_ops uio_sysfs_ops = {
+ .show = map_type_show,
+};
+
static struct kobj_type map_attr_type = {
.release = map_release,
+ .sysfs_ops = &uio_sysfs_ops,
.default_attrs = attrs,
};

2008-02-19 12:12:22

by Hans J. Koch

[permalink] [raw]
Subject: Re: [PATCH] UIO: introduce sysfs_ops for map_attr_ktype

Am Tue, 19 Feb 2008 01:55:05 -0800
schrieb Brandon Philips <[email protected]>:

> This fixes two bugs with UIO that cropped up recently in -rc1
>
> 1) WARNING: at fs/sysfs/file.c:334 sysfs_open_file when trying to open
> a map addr/size file - complaining about missing sysfs_ops for
> ktype
> 2) Permission denied when reading uio/uio0/maps/map0/{addr,size} when
> files are mode S_IRUGO
>
> Also fix a typo: attr_attribute -> addr_attribute

Hi Brandon,
thanks a lot for that one! I've tested with 2.6.25-rc2 with a Hilscher
NetX PCI card, works without problems. All attributes are accessible.
Makes 2.6.25-rc usable again :-)

Thanks,
Hans

>
> Signed-off-by: Brandon Philips <[email protected]>

Signed-off-by: Hans J. Koch <[email protected]>


>
> ---
> drivers/uio/uio.c | 54
> ++++++++++++++++++++++++++++++--------------- 1 file changed, 37
> insertions(+), 17 deletions(-)
>
> Index: linux-2.6/drivers/uio/uio.c
> ===================================================================
> --- linux-2.6.orig/drivers/uio/uio.c
> +++ linux-2.6/drivers/uio/uio.c
> @@ -57,29 +57,29 @@ struct uio_map {
> };
> #define to_map(map) container_of(map, struct uio_map, kobj)
>
> -
> -static ssize_t map_attr_show(struct kobject *kobj, struct
> kobj_attribute *attr,
> - char *buf)
> +static ssize_t map_addr_show(struct uio_mem *mem, char *buf)
> {
> - struct uio_map *map = to_map(kobj);
> - struct uio_mem *mem = map->mem;
> -
> - if (strncmp(attr->attr.name, "addr", 4) == 0)
> - return sprintf(buf, "0x%lx\n", mem->addr);
> -
> - if (strncmp(attr->attr.name, "size", 4) == 0)
> - return sprintf(buf, "0x%lx\n", mem->size);
> + return sprintf(buf, "0x%lx\n", mem->addr);
> +}
>
> - return -ENODEV;
> +static ssize_t map_size_show(struct uio_mem *mem, char *buf)
> +{
> + return sprintf(buf, "0x%lx\n", mem->size);
> }
>
> -static struct kobj_attribute attr_attribute =
> - __ATTR(addr, S_IRUGO, map_attr_show, NULL);
> -static struct kobj_attribute size_attribute =
> - __ATTR(size, S_IRUGO, map_attr_show, NULL);
> +struct uio_sysfs_entry {
> + struct attribute attr;
> + ssize_t (*show)(struct uio_mem *, char *);
> + ssize_t (*store)(struct uio_mem *, const char *, size_t);
> +};
> +
> +static struct uio_sysfs_entry addr_attribute =
> + __ATTR(addr, S_IRUGO, map_addr_show, NULL);
> +static struct uio_sysfs_entry size_attribute =
> + __ATTR(size, S_IRUGO, map_size_show, NULL);
>
> static struct attribute *attrs[] = {
> - &attr_attribute.attr,
> + &addr_attribute.attr,
> &size_attribute.attr,
> NULL, /* need to NULL terminate the list of
> attributes */ };
> @@ -90,8 +90,28 @@ static void map_release(struct kobject *
> kfree(map);
> }
>
> +static ssize_t map_type_show(struct kobject *kobj, struct attribute
> *attr,
> + char *buf)
> +{
> + struct uio_map *map = to_map(kobj);
> + struct uio_mem *mem = map->mem;
> + struct uio_sysfs_entry *entry;
> +
> + entry = container_of(attr, struct uio_sysfs_entry, attr);
> +
> + if (!entry->show)
> + return -EIO;
> +
> + return entry->show(mem, buf);
> +}
> +
> +static struct sysfs_ops uio_sysfs_ops = {
> + .show = map_type_show,
> +};
> +
> static struct kobj_type map_attr_type = {
> .release = map_release,
> + .sysfs_ops = &uio_sysfs_ops,
> .default_attrs = attrs,
> };

2008-02-20 06:50:50

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] UIO: introduce sysfs_ops for map_attr_ktype

On Tue, Feb 19, 2008 at 01:55:05AM -0800, Brandon Philips wrote:
> This fixes two bugs with UIO that cropped up recently in -rc1
>
> 1) WARNING: at fs/sysfs/file.c:334 sysfs_open_file when trying to open
> a map addr/size file - complaining about missing sysfs_ops for ktype
>
> 2) Permission denied when reading uio/uio0/maps/map0/{addr,size} when
> files are mode S_IRUGO
>
> Also fix a typo: attr_attribute -> addr_attribute
>
> Signed-off-by: Brandon Philips <[email protected]>

Hm, I thought I could get away with just using a kobject attribute, not
a special one.

Thanks for fixing this, it was my fault.

greg k-h