From: Veaceslav Falico <vfalico@redhat.com>
To: linux-pci@vger.kernel.org
Cc: torvalds@linux-foundation.org, tglx@linutronix.de,
yinghai@kernel.org, Knut_Petersen@t-online.de, mingo@kernel.org,
paulmck@linux.vnet.ibm.com, fweisbec@gmail.com,
linux-kernel@vger.kernel.org,
Veaceslav Falico <vfalico@redhat.com>,
Bjorn Helgaas <bhelgaas@google.com>,
Neil Horman <nhorman@tuxdriver.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: [PATCH v3 1/3] msi: free msi_desc entry only after we've released the kobject
Date: Tue, 29 Oct 2013 11:30:30 +0100 [thread overview]
Message-ID: <1383042632-7102-2-git-send-email-vfalico@redhat.com> (raw)
In-Reply-To: <1383042632-7102-1-git-send-email-vfalico@redhat.com>
Currently, we first do kobject_put(&entry->kobj) and the kfree(entry),
however kobject_put() doesn't guarantee us that it was the last reference
and that the kobj isn't used currently by someone else, so after we
kfree(entry) with the struct kobject - other users will begin using the
freed memory, instead of the actual kobject.
Fix this by using the kobject->release callback, which is called last when
the kobject is indeed not used and is cleaned up - it's msi_kobj_release(),
which can do the kfree(entry) safely (kobject_put/cleanup doesn't use the
kobj itself after ->release() was called, so we're safe).
In case we've failed to create the sysfs directories - just kfree()
it - cause we don't have the kobjects attached.
Also, remove the same functionality from populate_msi_sysfs(), cause on
failure we anyway call free_msi_irqs(), which will take care of all the
kobjects properly.
And add the forgotten pci_dev_put(pdev) in case of failure to register the
kobject in populate_msi_sysfs().
CC: Bjorn Helgaas <bhelgaas@google.com>
CC: Neil Horman <nhorman@tuxdriver.com>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: linux-pci@vger.kernel.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
drivers/pci/msi.c | 31 ++++++++++++-------------------
1 file changed, 12 insertions(+), 19 deletions(-)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index d5f90d6..5d70f49 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -374,19 +374,22 @@ static void free_msi_irqs(struct pci_dev *dev)
iounmap(entry->mask_base);
}
+ list_del(&entry->list);
+
/*
* Its possible that we get into this path
* When populate_msi_sysfs fails, which means the entries
* were not registered with sysfs. In that case don't
- * unregister them.
+ * unregister them, and just free. Otherwise the
+ * kobject->release will take care of freeing the entry via
+ * msi_kobj_release().
*/
if (entry->kobj.parent) {
kobject_del(&entry->kobj);
kobject_put(&entry->kobj);
+ } else {
+ kfree(entry);
}
-
- list_del(&entry->list);
- kfree(entry);
}
}
@@ -509,6 +512,7 @@ static void msi_kobj_release(struct kobject *kobj)
struct msi_desc *entry = to_msi_desc(kobj);
pci_dev_put(entry->dev);
+ kfree(entry);
}
static struct kobj_type msi_irq_ktype = {
@@ -522,7 +526,6 @@ static int populate_msi_sysfs(struct pci_dev *pdev)
struct msi_desc *entry;
struct kobject *kobj;
int ret;
- int count = 0;
pdev->msi_kset = kset_create_and_add("msi_irqs", NULL, &pdev->dev.kobj);
if (!pdev->msi_kset)
@@ -534,23 +537,13 @@ 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)
- goto out_unroll;
-
- count++;
+ if (ret) {
+ pci_dev_put(pdev);
+ return ret;
+ }
}
return 0;
-
-out_unroll:
- list_for_each_entry(entry, &pdev->msi_list, list) {
- if (!count)
- break;
- kobject_del(&entry->kobj);
- kobject_put(&entry->kobj);
- count--;
- }
- return ret;
}
/**
--
1.8.4
next parent reply other threads:[~2013-10-29 10:39 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1383042632-7102-1-git-send-email-vfalico@redhat.com>
2013-10-29 10:30 ` Veaceslav Falico [this message]
2013-10-29 11:32 ` Neil Horman
2013-10-29 16:34 ` Linus Torvalds
2013-10-29 16:47 ` Veaceslav Falico
2013-10-29 21:49 ` Greg Kroah-Hartman
2013-11-25 23:12 ` Bjorn Helgaas
2013-11-25 23:29 ` Greg Kroah-Hartman
2013-11-26 12:47 ` Neil Horman
2013-10-29 10:30 ` [PATCH v3 2/3] pci: remove redundant pci_dev_get/put() on kobject (un)register Veaceslav Falico
2013-10-29 11:34 ` Neil Horman
2013-10-29 10:30 ` [PATCH v3 3/3] msi: always unregister ->msi_kset within free_msi_irqs() Veaceslav Falico
2013-10-29 11:45 ` Neil Horman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1383042632-7102-2-git-send-email-vfalico@redhat.com \
--to=vfalico@redhat.com \
--cc=Knut_Petersen@t-online.de \
--cc=bhelgaas@google.com \
--cc=fweisbec@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=nhorman@tuxdriver.com \
--cc=paulmck@linux.vnet.ibm.com \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=yinghai@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®