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
| ||
|
Date: Fri, 18 Jan 2008 10:19:33 +0100 From: Cornelia Huck <cornelia.huck@...ibm.com> To: "Dave Young" <hidave.darkstar@...il.com> Cc: "Gabor Gombas" <gombasg@...aki.hu>, "Tejun Heo" <htejun@...il.com>, "Al Viro" <viro@...iv.linux.org.uk>, linux-kernel@...r.kernel.org, bluez-devel@...ts.sf.net, kay.sievers@...y.org, "Greg KH" <greg@...ah.com>, "Marcel Holtmann" <marcel@...tmann.org>, davem@...emloft.net Subject: Re: [Bluez-devel] Oops involving RFCOMM and sysfs On Fri, 18 Jan 2008 11:37:21 +0800, "Dave Young" <hidave.darkstar@...il.com> wrote: > Lets see the device_move function, seems there's some problems in it: > > 1302 int device_move(struct device *dev, struct device *new_parent) > 1303 { > 1304 int error; > 1305 struct device *old_parent; > 1306 struct kobject *new_parent_kobj; > 1307 > 1308 dev = get_device(dev); > 1309 if (!dev) > 1310 return -EINVAL; > 1311 > 1312 new_parent = get_device(new_parent); > 1313 new_parent_kobj = get_device_parent (dev, new_parent); > > Here could get kobject reference Eww. get_device_parent() may inflate the refcount in one case for !CONFIG_SYSFS_DEPRECATED, but often won't. (And the function is named confusingly, since it hints that we always get a reference, which we don't.) > > 1314 if (IS_ERR(new_parent_kobj)) { > 1315 error = PTR_ERR(new_parent_kobj); > 1316 put_device(new_parent); > 1317 goto out; > 1318 } > 1319 pr_debug("DEVICE: moving '%s' to '%s'\n", dev->bus_id, > 1320 new_parent ? new_parent->bus_id : "<NULL>"); > 1321 error = kobject_move(&dev->kobj, new_parent_kobj); > 1322 if (error) { > 1323 put_device(new_parent); > > imagine new_parent is NULL, then the new_parent_kobj should be put No, we would need a put_device_parent() (crappy name) which puts the reference iff get_device_parent() grabbed it. > > 1324 goto out; > 1325 } > 1326 old_parent = dev->parent; > 1327 dev->parent = new_parent; > 1328 if (old_parent) > 1329 klist_remove(&dev->knode_parent); > 1330 if (new_parent) > 1331 klist_add_tail(&dev->knode_parent, > &new_parent->klist_children); > 1332 if (!dev->class) > 1333 goto out_put; > > Why not put new_parent | new_parent_kobj? Because that is the good case :) > > 1334 error = device_move_class_links(dev, old_parent, new_parent); > 1335 if (error) { > 1336 /* We ignore errors on cleanup since we're hosed > anyway... */ > 1337 device_move_class_links(dev, new_parent, old_parent); > 1338 if (!kobject_move(&dev->kobj, &old_parent->kobj)) { > 1339 if (new_parent) > 1340 klist_remove(&dev->knode_parent); > 1341 if (old_parent) > 1342 klist_add_tail(&dev->knode_parent, > 1343 > &old_parent->klist_children); > 1344 } > 1345 put_device(new_parent); > > Same doubt as above We'd need put_device_parent() or whatever here as well, I guess. > > 1346 goto out; > 1347 } > 1348 out_put: > 1349 put_device(old_parent); > 1350 out: > 1351 put_device(dev); > 1352 return error; > 1353 } > > Hope I'm wrong, but if it's indeed bugs, I will send a patch about this. There are more problems, I'm afraid :( setup_parent() calls get_device_parent() as well, so device_add() has the same problems on error cleanup... I'll take a look at it if I find some time, but I'm afraid I'll not be able to do so before next week. -- 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/
Powered by blists - more mailing lists