UIO: fix kobject usage

The uio kobject code is "wierd". This patch should hopefully fix it up to be sane and not leak memory anymore. Cc: Kay Sievers <kay.sievers@vrfy.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Benedikt Spranger <b.spranger@linutronix.de> Signed-off-by: Hans J. Koch <hjk@linutronix.de> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

UIO: fix kobject usage
The uio kobject code is "wierd". This patch should hopefully fix it up to be sane and not leak memory anymore. Cc: Kay Sievers <kay.sievers@vrfy.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Benedikt Spranger <b.spranger@linutronix.de> Signed-off-by: Hans J. Koch <hjk@linutronix.de> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Greg Kroah-Hartman
1 parent 9f66fa2a46
Showing 2 changed files with 52 additions and 45 deletions Side-by-side Diff
drivers/uio/uio.c
include/linux/uio_driver.h
@@ -34,7 +34,7 @@
 	wait_queue_head_t	wait;
 	int			vma_count;
 	struct uio_info		*info;
-	struct kset 		map_attr_kset;
+	struct kobject		*map_dir;
 };
  
 static int uio_major;
  
  
  
  
  
  
  
  
  
  
@@ -51,47 +51,48 @@
  * attributes
  */
  
-static struct attribute attr_addr = {
-	.name  = "addr",
-	.mode  = S_IRUGO,
+struct uio_map {
+	struct kobject kobj;
+	struct uio_mem *mem;
 };
+#define to_map(map) container_of(map, struct uio_map, kobj)
  
-static struct attribute attr_size = {
-	.name  = "size",
-	.mode  = S_IRUGO,
-};
  
-static struct attribute* map_attrs[] = {
-	&attr_addr, &attr_size, NULL
-};
-
-static ssize_t map_attr_show(struct kobject *kobj, struct attribute *attr,
+static ssize_t map_attr_show(struct kobject *kobj, struct kobj_attribute *attr,
 			     char *buf)
 {
-	struct uio_mem *mem = container_of(kobj, struct uio_mem, kobj);
+	struct uio_map *map = to_map(kobj);
+	struct uio_mem *mem = map->mem;
  
-	if (strncmp(attr->name,"addr",4) == 0)
+	if (strncmp(attr->attr.name, "addr", 4) == 0)
 		return sprintf(buf, "0x%lx\n", mem->addr);
  
-	if (strncmp(attr->name,"size",4) == 0)
+	if (strncmp(attr->attr.name, "size", 4) == 0)
 		return sprintf(buf, "0x%lx\n", mem->size);
  
 	return -ENODEV;
 }
  
-static void map_attr_release(struct kobject *kobj)
-{
-	/* TODO ??? */
-}
+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);
  
-static struct sysfs_ops map_attr_ops = {
-	.show  = map_attr_show,
+static struct attribute *attrs[] = {
+	&attr_attribute.attr,
+	&size_attribute.attr,
+	NULL,	/* need to NULL terminate the list of attributes */
 };
  
+static void map_release(struct kobject *kobj)
+{
+	struct uio_map *map = to_map(kobj);
+	kfree(map);
+}
+
 static struct kobj_type map_attr_type = {
-	.release	= map_attr_release,
-	.sysfs_ops	= &map_attr_ops,
-	.default_attrs	= map_attrs,
+	.release	= map_release,
+	.default_attrs	= attrs,
 };
  
 static ssize_t show_name(struct device *dev,
@@ -148,6 +149,7 @@
 	int mi;
 	int map_found = 0;
 	struct uio_mem *mem;
+	struct uio_map *map;
  
 	ret = sysfs_create_group(&idev->dev->kobj, &uio_attr_grp);
 	if (ret)
  
  
  
  
  
@@ -159,31 +161,34 @@
 			break;
 		if (!map_found) {
 			map_found = 1;
-			kobject_set_name(&idev->map_attr_kset.kobj,"maps");
-			idev->map_attr_kset.kobj.ktype = &map_attr_type;
-			idev->map_attr_kset.kobj.parent = &idev->dev->kobj;
-			ret = kset_register(&idev->map_attr_kset);
-			if (ret)
-				goto err_remove_group;
+			idev->map_dir = kobject_create_and_add("maps",
+							&idev->dev->kobj);
+			if (!idev->map_dir)
+				goto err;
 		}
-		kobject_init(&mem->kobj);
-		kobject_set_name(&mem->kobj,"map%d",mi);
-		mem->kobj.parent = &idev->map_attr_kset.kobj;
-		mem->kobj.kset = &idev->map_attr_kset;
-		ret = kobject_add(&mem->kobj);
+		map = kzalloc(sizeof(*map), GFP_KERNEL);
+		if (!map)
+			goto err;
+		kobject_init_ng(&map->kobj, &map_attr_type);
+		map->mem = mem;
+		mem->map = map;
+		ret = kobject_add_ng(&map->kobj, idev->map_dir, "map%d", mi);
 		if (ret)
-			goto err_remove_maps;
+			goto err;
+		ret = kobject_uevent(&map->kobj, KOBJ_ADD);
+		if (ret)
+			goto err;
 	}
  
 	return 0;
  
-err_remove_maps:
+err:
 	for (mi--; mi>=0; mi--) {
 		mem = &idev->info->mem[mi];
-		kobject_unregister(&mem->kobj);
+		map = mem->map;
+		kobject_unregister(&map->kobj);
 	}
-	kset_unregister(&idev->map_attr_kset); /* Needed ? */
-err_remove_group:
+	kobject_unregister(idev->map_dir);
 	sysfs_remove_group(&idev->dev->kobj, &uio_attr_grp);
 err_group:
 	dev_err(idev->dev, "error creating sysfs files (%d)\n", ret);
  
@@ -198,9 +203,9 @@
 		mem = &idev->info->mem[mi];
 		if (mem->size == 0)
 			break;
-		kobject_unregister(&mem->kobj);
+		kobject_unregister(&mem->map->kobj);
 	}
-	kset_unregister(&idev->map_attr_kset);
+	kobject_unregister(idev->map_dir);
 	sysfs_remove_group(&idev->dev->kobj, &uio_attr_grp);
 }
  
@@ -18,20 +18,22 @@
 #include <linux/fs.h>
 #include <linux/interrupt.h>
  
+struct uio_map;
+
 /**
  * struct uio_mem - description of a UIO memory region
- * @kobj:		kobject for this mapping
  * @addr:		address of the device's memory
  * @size:		size of IO
  * @memtype:		type of memory addr points to
  * @internal_addr:	ioremap-ped version of addr, for driver internal use
+ * @map:		for use by the UIO core only.
  */
 struct uio_mem {
-	struct kobject		kobj;
 	unsigned long		addr;
 	unsigned long		size;
 	int			memtype;
 	void __iomem		*internal_addr;
+	struct uio_map		*map;
 };
  
 #define MAX_UIO_MAPS 	5
...	...	@@ -34,7 +34,7 @@
34	34	wait_queue_head_t wait;
35	35	int vma_count;
36	36	struct uio_info *info;
37		- struct kset map_attr_kset;
	37	+ struct kobject *map_dir;
38	38	};
39	39
40	40	static int uio_major;
41	41
42	42
43	43
44	44
45	45
46	46
47	47
48	48
49	49
50	50
...	...	@@ -51,47 +51,48 @@
51	51	* attributes
52	52	*/
53	53
54		-static struct attribute attr_addr = {
55		- .name = "addr",
56		- .mode = S_IRUGO,
	54	+struct uio_map {
	55	+ struct kobject kobj;
	56	+ struct uio_mem *mem;
57	57	};
	58	+#define to_map(map) container_of(map, struct uio_map, kobj)
58	59
59		-static struct attribute attr_size = {
60		- .name = "size",
61		- .mode = S_IRUGO,
62		-};
63	60
64		-static struct attribute* map_attrs[] = {
65		- &attr_addr, &attr_size, NULL
66		-};
67		-
68		-static ssize_t map_attr_show(struct kobject kobj, struct attribute attr,
	61	+static ssize_t map_attr_show(struct kobject kobj, struct kobj_attribute attr,
69	62	char *buf)
70	63	{
71		- struct uio_mem *mem = container_of(kobj, struct uio_mem, kobj);
	64	+ struct uio_map *map = to_map(kobj);
	65	+ struct uio_mem *mem = map->mem;
72	66
73		- if (strncmp(attr->name,"addr",4) == 0)
	67	+ if (strncmp(attr->attr.name, "addr", 4) == 0)
74	68	return sprintf(buf, "0x%lx\n", mem->addr);
75	69
76		- if (strncmp(attr->name,"size",4) == 0)
	70	+ if (strncmp(attr->attr.name, "size", 4) == 0)
77	71	return sprintf(buf, "0x%lx\n", mem->size);
78	72
79	73	return -ENODEV;
80	74	}
81	75
82		-static void map_attr_release(struct kobject *kobj)
83		-{
84		- /* TODO ??? */
85		-}
	76	+static struct kobj_attribute attr_attribute =
	77	+ __ATTR(addr, S_IRUGO, map_attr_show, NULL);
	78	+static struct kobj_attribute size_attribute =
	79	+ __ATTR(size, S_IRUGO, map_attr_show, NULL);
86	80
87		-static struct sysfs_ops map_attr_ops = {
88		- .show = map_attr_show,
	81	+static struct attribute *attrs[] = {
	82	+ &attr_attribute.attr,
	83	+ &size_attribute.attr,
	84	+ NULL, /* need to NULL terminate the list of attributes */
89	85	};
90	86
	87	+static void map_release(struct kobject *kobj)
	88	+{
	89	+ struct uio_map *map = to_map(kobj);
	90	+ kfree(map);
	91	+}
	92	+
91	93	static struct kobj_type map_attr_type = {
92		- .release = map_attr_release,
93		- .sysfs_ops = &map_attr_ops,
94		- .default_attrs = map_attrs,
	94	+ .release = map_release,
	95	+ .default_attrs = attrs,
95	96	};
96	97
97	98	static ssize_t show_name(struct device *dev,
...	...	@@ -148,6 +149,7 @@
148	149	int mi;
149	150	int map_found = 0;
150	151	struct uio_mem *mem;
	152	+ struct uio_map *map;
151	153
152	154	ret = sysfs_create_group(&idev->dev->kobj, &uio_attr_grp);
153	155	if (ret)
154	156
155	157
156	158
157	159
158	160
...	...	@@ -159,31 +161,34 @@
159	161	break;
160	162	if (!map_found) {
161	163	map_found = 1;
162		- kobject_set_name(&idev->map_attr_kset.kobj,"maps");
163		- idev->map_attr_kset.kobj.ktype = &map_attr_type;
164		- idev->map_attr_kset.kobj.parent = &idev->dev->kobj;
165		- ret = kset_register(&idev->map_attr_kset);
166		- if (ret)
167		- goto err_remove_group;
	164	+ idev->map_dir = kobject_create_and_add("maps",
	165	+ &idev->dev->kobj);
	166	+ if (!idev->map_dir)
	167	+ goto err;
168	168	}
169		- kobject_init(&mem->kobj);
170		- kobject_set_name(&mem->kobj,"map%d",mi);
171		- mem->kobj.parent = &idev->map_attr_kset.kobj;
172		- mem->kobj.kset = &idev->map_attr_kset;
173		- ret = kobject_add(&mem->kobj);
	169	+ map = kzalloc(sizeof(*map), GFP_KERNEL);
	170	+ if (!map)
	171	+ goto err;
	172	+ kobject_init_ng(&map->kobj, &map_attr_type);
	173	+ map->mem = mem;
	174	+ mem->map = map;
	175	+ ret = kobject_add_ng(&map->kobj, idev->map_dir, "map%d", mi);
174	176	if (ret)
175		- goto err_remove_maps;
	177	+ goto err;
	178	+ ret = kobject_uevent(&map->kobj, KOBJ_ADD);
	179	+ if (ret)
	180	+ goto err;
176	181	}
177	182
178	183	return 0;
179	184
180		-err_remove_maps:
	185	+err:
181	186	for (mi--; mi>=0; mi--) {
182	187	mem = &idev->info->mem[mi];
183		- kobject_unregister(&mem->kobj);
	188	+ map = mem->map;
	189	+ kobject_unregister(&map->kobj);
184	190	}
185		- kset_unregister(&idev->map_attr_kset); /* Needed ? */
186		-err_remove_group:
	191	+ kobject_unregister(idev->map_dir);
187	192	sysfs_remove_group(&idev->dev->kobj, &uio_attr_grp);
188	193	err_group:
189	194	dev_err(idev->dev, "error creating sysfs files (%d)\n", ret);
190	195
...	...	@@ -198,9 +203,9 @@
198	203	mem = &idev->info->mem[mi];
199	204	if (mem->size == 0)
200	205	break;
201		- kobject_unregister(&mem->kobj);
	206	+ kobject_unregister(&mem->map->kobj);
202	207	}
203		- kset_unregister(&idev->map_attr_kset);
	208	+ kobject_unregister(idev->map_dir);
204	209	sysfs_remove_group(&idev->dev->kobj, &uio_attr_grp);
205	210	}
206	211
...	...	@@ -18,20 +18,22 @@
18	18	#include <linux/fs.h>
19	19	#include <linux/interrupt.h>
20	20
	21	+struct uio_map;
	22	+
21	23	/**
22	24	* struct uio_mem - description of a UIO memory region
23		- * @kobj: kobject for this mapping
24	25	* @addr: address of the device's memory
25	26	* @size: size of IO
26	27	* @memtype: type of memory addr points to
27	28	* @internal_addr: ioremap-ped version of addr, for driver internal use
	29	+ * @map: for use by the UIO core only.
28	30	*/
29	31	struct uio_mem {
30		- struct kobject kobj;
31	32	unsigned long addr;
32	33	unsigned long size;
33	34	int memtype;
34	35	void __iomem *internal_addr;
	36	+ struct uio_map *map;
35	37	};
36	38
37	39	#define MAX_UIO_MAPS 5