[+cc Neil (he added this code in da8d1c8ba4), Greg]
On Mon, Sep 16, 2013 at 7:47 PM, Veaceslav Falico <vfalico@xxxxxxxxxx> wrote:
> Before trying to kobject_init_and_add(), we add a reference to pdev via
> pci_dev_get(pdev). However, if it fails to init and/or add the kobject, we
> don't return it back - even on out_unroll.
>
> Fix this by adding pci_dev_put(pdev) before going to unrolling section.
>
> CC: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> CC: linux-pci@xxxxxxxxxxxxxxx
> CC: linux-kernel@xxxxxxxxxxxxxxx
> Signed-off-by: Veaceslav Falico <vfalico@xxxxxxxxxx>
> ---
> drivers/pci/msi.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index d5f90d6..14bf578 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -534,8 +534,10 @@ static int populate_msi_sysfs(struct pci_dev *pdev)
> pci_dev_get(pdev);
> ret = kobject_init_and_add(kobj, &msi_irq_ktype, NULL,
> "%u", entry->irq);
> - if (ret)
> + if (ret) {
> + pci_dev_put(pdev);
> goto out_unroll;
> + }
>
> count++;
> }
I don't understand why this code does the pci_dev_get() in the first
place. The pdev->msi_list of msi_desc structs is private to the
pci_dev, and even without bumping the refcount, there should be no way
for the pci_dev to be freed before the msi_desc.
I also don't understand this nearby code (the same pattern appears in
free_msi_irqs()):
out_unroll:
list_for_each_entry(entry, &pdev->msi_list, list) {
if (!count)
break;
kobject_del(&entry->kobj);
kobject_put(&entry->kobj);
count--;
}
Why do we call kobject_del() here? The kobject_put() will call
kobject_del() anyway, so it looks redundant.
Documentation/kobject.txt says kobject_del() must be called explicitly
to break a circular reference, but I don't think we have that here.
Also, I think it is incorrect that free_msi_irqs() does this:
if (entry->kobj.parent) {
kobject_del(&entry->kobj);
kobject_put(&entry->kobj);
}
list_del(&entry->list);
kfree(entry);
I think the "kfree(entry)" should be in msi_kobj_release() instead.
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
