On 09/20/2010 05:55 PM, James Bottomley wrote: > On Mon, 2010-09-20 at 17:42 +0200, Boaz Harrosh wrote: >> I think I have a compromise. If it is indeed the dev_set_name() leak >> then we can just deallocate the name on the error return path. Therefore >> any drivers that have the device embedded and rely on it been freed without >> calling _put will be fine as before. And these calling _put will be fine >> as well. >> >> See below >> Boaz >> --- >> git diff --stat -p -M drivers/base/core.c >> drivers/base/core.c | 1 + >> 1 files changed, 1 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/base/core.c b/drivers/base/core.c >> index d1b2c9a..054fac2 100644 >> --- a/drivers/base/core.c >> +++ b/drivers/base/core.c >> @@ -1068,6 +1068,7 @@ done: >> if (parent) >> put_device(parent); >> name_error: >> + kfree(dev->kobj.name); >> kfree(dev->p); >> dev->p = NULL; >> goto done; > > That's not really good enough ... it will result in a double free > because the final put (if there is one) will free the name again. > > This would work, but it feels a bit wrong to be poking at the kobject > from within the driver core ... perhaps a kobject_free_name() call might > be better? > It would not be the first place to directly access kobj. members but sure that will work as well. > James > > --- > > diff --git a/drivers/base/core.c b/drivers/base/core.c > index d1b2c9a..88ffaca 100644 > --- a/drivers/base/core.c > +++ b/drivers/base/core.c > @@ -1067,6 +1067,8 @@ done: > cleanup_device_parent(dev); > if (parent) > put_device(parent); > + kfree(dev->kobj.name); > + dev->kobj.name = NULL; Rrr thanks, the NULL. Therefore the lack of a signed-off ;-) I take it that you agree with me that for some code it would be nice if we add support for not mandating a device_put after a device_register for the embedded types? For sake of already written code? > name_error: > kfree(dev->p); > dev->p = NULL; > > Thanks Boaz -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
