| Message ID | 20200528021322.1984-1-wu000273@umn.edu (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Commit | 8a94644b440eef5a7b9c104ac8aa7a7f413e35e5 |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | Fix reference count leak in pci_create_slot | expand |
On Wed, May 27, 2020 at 09:13:22PM -0500, wu000273@umn.edu wrote: > From: Qiushi Wu <wu000273@umn.edu> > > kobject_init_and_add() takes reference even when it fails. > If this function returns an error, kobject_put() must be called to > properly clean up the memory associated with the object. Thus, > when call of kobject_init_and_add() fail, we should call kobject_put() > instead of kfree(). Previous commit "b8eb718348b8" fixed a similar problem. Looks like you are also checking other kobject_init_and_add() callers? I see some similar messages on linux-kernel. Thanks for doing that! I looked at the first dozen or so callers and the following look broken and I don't see patches for them (yet): cache_add_dev() sq_dev_add blk_integrity_add acpi_expose_nondev_subnodes rnbd_clt_add_dev_kobj > Fixes: 5fe6cc60680d ("PCI: prevent duplicate slot names") I'm not sure this is correct. 5fe6cc60680d didn't *add* a kobject_init_and_add() call; it was there before. And even before 5fe6cc60680d there was no kobject_put() in the error path. Am I missing something? For now, I applied this to pci/hotplug for v5.9. I'll update the commit log to add the Fixes tag if necessary. > Signed-off-by: Qiushi Wu <wu000273@umn.edu> > --- > drivers/pci/slot.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c > index cc386ef2fa12..3861505741e6 100644 > --- a/drivers/pci/slot.c > +++ b/drivers/pci/slot.c > @@ -268,13 +268,16 @@ struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr, > slot_name = make_slot_name(name); > if (!slot_name) { > err = -ENOMEM; > + kfree(slot); > goto err; > } > > err = kobject_init_and_add(&slot->kobj, &pci_slot_ktype, NULL, > "%s", slot_name); > - if (err) > + if (err) { > + kobject_put(&slot->kobj); > goto err; > + } > > INIT_LIST_HEAD(&slot->list); > list_add(&slot->list, &parent->slots); > @@ -293,7 +296,6 @@ struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr, > mutex_unlock(&pci_slot_mutex); > return slot; > err: > - kfree(slot); > slot = ERR_PTR(err); > goto out; > } > -- > 2.17.1 >
diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c index cc386ef2fa12..3861505741e6 100644 --- a/drivers/pci/slot.c +++ b/drivers/pci/slot.c @@ -268,13 +268,16 @@ struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr, slot_name = make_slot_name(name); if (!slot_name) { err = -ENOMEM; + kfree(slot); goto err; } err = kobject_init_and_add(&slot->kobj, &pci_slot_ktype, NULL, "%s", slot_name); - if (err) + if (err) { + kobject_put(&slot->kobj); goto err; + } INIT_LIST_HEAD(&slot->list); list_add(&slot->list, &parent->slots); @@ -293,7 +296,6 @@ struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr, mutex_unlock(&pci_slot_mutex); return slot; err: - kfree(slot); slot = ERR_PTR(err); goto out; }