aboutsummaryrefslogtreecommitdiffstats
path: root/drivers/base
diff options
context:
space:
mode:
authorPatrick Mochel <mochel@digitalimplant.org>2005-04-06 02:46:33 -0400
committerGreg Kroah-Hartman <gregkh@suse.de>2005-06-20 18:15:27 -0400
commit0d3e5a2e39b6ba2974e9e7c2a429018c45de8e76 (patch)
tree30e584b73c356adce49dcc9df75332abaef95470 /drivers/base
parentb86c1df1f98d16c999423a3907eb40a9423f481e (diff)
[PATCH] Driver Core: fix bk-driver-core kills ppc64
There's no check to see if the device is already bound to a driver, which could do bad things. The first thing to go wrong is that it will try to match a driver with a device already bound to one. In some cases (it appears with USB with drivers/usb/core/usb.c::usb_match_id()), some drivers will match a device based on the class type, so it would be common (especially for HID devices) to match a device that is already bound. The fun comes when ->probe() is called, it fails, then driver_probe_device() does this: dev->driver = NULL; Later on, that pointer could be be dereferenced without checking and cause hell to break loose. This problem could be nasty. It's very hardware dependent, since some devices could have a different set of matching qualifiers than others. Now, I don't quite see exactly where/how you were getting that crash. You're dereferencing bad memory, but I'm not sure which pointer was bad and where it came from, but it could have come from a couple of different places. The patch below will hopefully fix it all up for you. It's against 2.6.12-rc2-mm1, and does the following: - Move logic to driver_probe_device() and comments uncommon returns: 1 - If device is bound 0 - If device not bound, and no error error - If there was an error. - Move locking to caller of that function, since we want to lock a device for the entire time we're trying to bind it to a driver (to prevent against a driver being loaded at the same time). - Update __device_attach() and __driver_attach() to do that locking. - Check if device is already bound in __driver_attach() - Update the converse device_release_driver() so it locks the device around all of the operations. - Mark driver_probe_device() as static and remove export. It's an internal function, it should stay that way, and there are no other callers. If there is ever a need to export it, we can audit it as necessary. Signed-off-by: Andrew Morton <akpm@osdl.org>
Diffstat (limited to 'drivers/base')
-rw-r--r--drivers/base/dd.c142
1 files changed, 74 insertions, 68 deletions
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index dd2a8a79c121..8510918109e0 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -35,6 +35,8 @@
35 * nor take the bus's rwsem. Please verify those are accounted 35 * nor take the bus's rwsem. Please verify those are accounted
36 * for before calling this. (It is ok to call with no other effort 36 * for before calling this. (It is ok to call with no other effort
37 * from a driver's probe() method.) 37 * from a driver's probe() method.)
38 *
39 * This function must be called with @dev->sem held.
38 */ 40 */
39void device_bind_driver(struct device * dev) 41void device_bind_driver(struct device * dev)
40{ 42{
@@ -57,54 +59,56 @@ void device_bind_driver(struct device * dev)
57 * because we don't know the format of the ID structures, nor what 59 * because we don't know the format of the ID structures, nor what
58 * is to be considered a match and what is not. 60 * is to be considered a match and what is not.
59 * 61 *
60 * If we find a match, we call @drv->probe(@dev) if it exists, and 62 *
61 * call device_bind_driver() above. 63 * This function returns 1 if a match is found, an error if one
64 * occurs (that is not -ENODEV or -ENXIO), and 0 otherwise.
65 *
66 * This function must be called with @dev->sem held.
62 */ 67 */
63int driver_probe_device(struct device_driver * drv, struct device * dev) 68static int driver_probe_device(struct device_driver * drv, struct device * dev)
64{ 69{
65 int error = 0; 70 int ret = 0;
66 71
67 if (drv->bus->match && !drv->bus->match(dev, drv)) 72 if (drv->bus->match && !drv->bus->match(dev, drv))
68 return -ENODEV; 73 goto Done;
69 74
70 down(&dev->sem); 75 pr_debug("%s: Matched Device %s with Driver %s\n",
76 drv->bus->name, dev->bus_id, drv->name);
71 dev->driver = drv; 77 dev->driver = drv;
72 if (drv->probe) { 78 if (drv->probe) {
73 error = drv->probe(dev); 79 ret = drv->probe(dev);
74 if (error) { 80 if (ret) {
75 dev->driver = NULL; 81 dev->driver = NULL;
76 up(&dev->sem); 82 goto ProbeFailed;
77 return error;
78 } 83 }
79 } 84 }
80 up(&dev->sem);
81 device_bind_driver(dev); 85 device_bind_driver(dev);
82 return 0; 86 ret = 1;
87 pr_debug("%s: Bound Device %s to Driver %s\n",
88 drv->bus->name, dev->bus_id, drv->name);
89 goto Done;
90
91 ProbeFailed:
92 if (ret == -ENODEV || ret == -ENXIO) {
93 /* Driver matched, but didn't support device
94 * or device not found.
95 * Not an error; keep going.
96 */
97 ret = 0;
98 } else {
99 /* driver matched but the probe failed */
100 printk(KERN_WARNING
101 "%s: probe of %s failed with error %d\n",
102 drv->name, dev->bus_id, ret);
103 }
104 Done:
105 return ret;
83} 106}
84 107
85static int __device_attach(struct device_driver * drv, void * data) 108static int __device_attach(struct device_driver * drv, void * data)
86{ 109{
87 struct device * dev = data; 110 struct device * dev = data;
88 int error; 111 return driver_probe_device(drv, dev);
89
90 error = driver_probe_device(drv, dev);
91 if (error) {
92 if ((error == -ENODEV) || (error == -ENXIO)) {
93 /* Driver matched, but didn't support device
94 * or device not found.
95 * Not an error; keep going.
96 */
97 error = 0;
98 } else {
99 /* driver matched but the probe failed */
100 printk(KERN_WARNING
101 "%s: probe of %s failed with error %d\n",
102 drv->name, dev->bus_id, error);
103 }
104 return error;
105 }
106 /* stop looking, this device is attached */
107 return 1;
108} 112}
109 113
110/** 114/**
@@ -114,37 +118,43 @@ static int __device_attach(struct device_driver * drv, void * data)
114 * Walk the list of drivers that the bus has and call 118 * Walk the list of drivers that the bus has and call
115 * driver_probe_device() for each pair. If a compatible 119 * driver_probe_device() for each pair. If a compatible
116 * pair is found, break out and return. 120 * pair is found, break out and return.
121 *
122 * Returns 1 if the device was bound to a driver; 0 otherwise.
117 */ 123 */
118int device_attach(struct device * dev) 124int device_attach(struct device * dev)
119{ 125{
126 int ret = 0;
127
128 down(&dev->sem);
120 if (dev->driver) { 129 if (dev->driver) {
121 device_bind_driver(dev); 130 device_bind_driver(dev);
122 return 1; 131 ret = 1;
123 } 132 } else
124 133 ret = bus_for_each_drv(dev->bus, NULL, dev, __device_attach);
125 return bus_for_each_drv(dev->bus, NULL, dev, __device_attach); 134 up(&dev->sem);
135 return ret;
126} 136}
127 137
128static int __driver_attach(struct device * dev, void * data) 138static int __driver_attach(struct device * dev, void * data)
129{ 139{
130 struct device_driver * drv = data; 140 struct device_driver * drv = data;
131 int error = 0; 141
132 142 /*
133 if (!dev->driver) { 143 * Lock device and try to bind to it. We drop the error
134 error = driver_probe_device(drv, dev); 144 * here and always return 0, because we need to keep trying
135 if (error) { 145 * to bind to devices and some drivers will return an error
136 if (error != -ENODEV) { 146 * simply if it didn't support the device.
137 /* driver matched but the probe failed */ 147 *
138 printk(KERN_WARNING 148 * driver_probe_device() will spit a warning if there
139 "%s: probe of %s failed with error %d\n", 149 * is an error.
140 drv->name, dev->bus_id, error); 150 */
141 } else 151
142 error = 0; 152 down(&dev->sem);
143 return error; 153 if (!dev->driver)
144 } 154 driver_probe_device(drv, dev);
145 /* stop looking, this driver is attached */ 155 up(&dev->sem);
146 return 1; 156
147 } 157
148 return 0; 158 return 0;
149} 159}
150 160
@@ -156,9 +166,6 @@ static int __driver_attach(struct device * dev, void * data)
156 * match the driver with each one. If driver_probe_device() 166 * match the driver with each one. If driver_probe_device()
157 * returns 0 and the @dev->driver is set, we've found a 167 * returns 0 and the @dev->driver is set, we've found a
158 * compatible pair. 168 * compatible pair.
159 *
160 * Note that we ignore the -ENODEV error from driver_probe_device(),
161 * since it's perfectly valid for a driver not to bind to any devices.
162 */ 169 */
163void driver_attach(struct device_driver * drv) 170void driver_attach(struct device_driver * drv)
164{ 171{
@@ -176,19 +183,19 @@ void driver_attach(struct device_driver * drv)
176 */ 183 */
177void device_release_driver(struct device * dev) 184void device_release_driver(struct device * dev)
178{ 185{
179 struct device_driver * drv = dev->driver; 186 struct device_driver * drv;
180
181 if (!drv)
182 return;
183
184 sysfs_remove_link(&drv->kobj, kobject_name(&dev->kobj));
185 sysfs_remove_link(&dev->kobj, "driver");
186 klist_del(&dev->knode_driver);
187 187
188 down(&dev->sem); 188 down(&dev->sem);
189 if (drv->remove) 189 if (dev->driver) {
190 drv->remove(dev); 190 drv = dev->driver;
191 dev->driver = NULL; 191 sysfs_remove_link(&drv->kobj, kobject_name(&dev->kobj));
192 sysfs_remove_link(&dev->kobj, "driver");
193 klist_del(&dev->knode_driver);
194
195 if (drv->remove)
196 drv->remove(dev);
197 dev->driver = NULL;
198 }
192 up(&dev->sem); 199 up(&dev->sem);
193} 200}
194 201
@@ -208,7 +215,6 @@ void driver_detach(struct device_driver * drv)
208} 215}
209 216
210 217
211EXPORT_SYMBOL_GPL(driver_probe_device);
212EXPORT_SYMBOL_GPL(device_bind_driver); 218EXPORT_SYMBOL_GPL(device_bind_driver);
213EXPORT_SYMBOL_GPL(device_release_driver); 219EXPORT_SYMBOL_GPL(device_release_driver);
214EXPORT_SYMBOL_GPL(device_attach); 220EXPORT_SYMBOL_GPL(device_attach);