diff mbox series

[v2] x86/PCI: fix a memory leak bug

Message ID 1555423298-6677-1-git-send-email-wang6495@umn.edu (mailing list archive)
State Superseded, archived
Headers show
Series [v2] x86/PCI: fix a memory leak bug | expand

Commit Message

Wenwen Wang April 16, 2019, 2:01 p.m. UTC
In pcibios_irq_init(), the PCI IRQ routing table 'pirq_table' is firstly
found through pirq_find_routing_table(). If the table is not found and
'CONFIG_PCI_BIOS' is defined, the table is then allocated in
pcibios_get_irq_routing_table() using kmalloc(). In the following
execution, if the I/O APIC is used, this table is actually not used.
However, in that case, the allocated table is not freed, which can lead to
a memory leak bug.

To fix this issue, this patch frees the allocated table if it is not used.

Signed-off-by: Wenwen Wang <wang6495@umn.edu>
---
 arch/x86/pci/irq.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Thomas Gleixner April 16, 2019, 7:58 p.m. UTC | #1
On Tue, 16 Apr 2019, Wenwen Wang wrote:

> In pcibios_irq_init(), the PCI IRQ routing table 'pirq_table' is firstly
> found through pirq_find_routing_table(). If the table is not found and
> 'CONFIG_PCI_BIOS' is defined, the table is then allocated in
> pcibios_get_irq_routing_table() using kmalloc(). In the following
> execution, if the I/O APIC is used, this table is actually not used.
> However, in that case, the allocated table is not freed, which can lead to
> a memory leak bug.

s/which can lead to/which is/

There is no 'can'. It simply is a memory leak.

> To fix this issue, this patch frees the allocated table if it is not used.

To fix this issue, free the allocated table if it is not used.

'this patch' is completely redundant information and discouraged in
Documentation/process/....

Other than that:

Acked-by: Thomas Gleixner <tglx@linutronix.de>
Wenwen Wang April 16, 2019, 9:29 p.m. UTC | #2
On Tue, Apr 16, 2019 at 3:33 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Tue, 16 Apr 2019, Wenwen Wang wrote:
>
> > In pcibios_irq_init(), the PCI IRQ routing table 'pirq_table' is firstly
> > found through pirq_find_routing_table(). If the table is not found and
> > 'CONFIG_PCI_BIOS' is defined, the table is then allocated in
> > pcibios_get_irq_routing_table() using kmalloc(). In the following
> > execution, if the I/O APIC is used, this table is actually not used.
> > However, in that case, the allocated table is not freed, which can lead to
> > a memory leak bug.
>
> s/which can lead to/which is/
>
> There is no 'can'. It simply is a memory leak.
>
> > To fix this issue, this patch frees the allocated table if it is not used.
>
> To fix this issue, free the allocated table if it is not used.
>
> 'this patch' is completely redundant information and discouraged in
> Documentation/process/....
>

Thanks for your suggestions, Thomas. I will revise the commit's message.

Wenwen

> Other than that:
>
> Acked-by: Thomas Gleixner <tglx@linutronix.de>
Ingo Molnar April 17, 2019, 5:58 a.m. UTC | #3
* Wenwen Wang <wang6495@umn.edu> wrote:

> On Tue, Apr 16, 2019 at 3:33 PM Thomas Gleixner <tglx@linutronix.de> wrote:
> >
> > On Tue, 16 Apr 2019, Wenwen Wang wrote:
> >
> > > In pcibios_irq_init(), the PCI IRQ routing table 'pirq_table' is firstly
> > > found through pirq_find_routing_table(). If the table is not found and
> > > 'CONFIG_PCI_BIOS' is defined, the table is then allocated in
> > > pcibios_get_irq_routing_table() using kmalloc(). In the following
> > > execution, if the I/O APIC is used, this table is actually not used.
> > > However, in that case, the allocated table is not freed, which can lead to
> > > a memory leak bug.
> >
> > s/which can lead to/which is/
> >
> > There is no 'can'. It simply is a memory leak.
> >
> > > To fix this issue, this patch frees the allocated table if it is not used.
> >
> > To fix this issue, free the allocated table if it is not used.
> >
> > 'this patch' is completely redundant information and discouraged in
> > Documentation/process/....
> >
> 
> Thanks for your suggestions, Thomas. I will revise the commit's message.
> 
> Wenwen
> 
> > Other than that:
> >
> > Acked-by: Thomas Gleixner <tglx@linutronix.de>

You didn't add Thomas's Acked-by to your commit ...

Thanks,

	Ingo
Wenwen Wang April 17, 2019, 2:10 p.m. UTC | #4
On Wed, Apr 17, 2019 at 12:58 AM Ingo Molnar <mingo@kernel.org> wrote:
>
>
> * Wenwen Wang <wang6495@umn.edu> wrote:
>
> > On Tue, Apr 16, 2019 at 3:33 PM Thomas Gleixner <tglx@linutronix.de> wrote:
> > >
> > > On Tue, 16 Apr 2019, Wenwen Wang wrote:
> > >
> > > > In pcibios_irq_init(), the PCI IRQ routing table 'pirq_table' is firstly
> > > > found through pirq_find_routing_table(). If the table is not found and
> > > > 'CONFIG_PCI_BIOS' is defined, the table is then allocated in
> > > > pcibios_get_irq_routing_table() using kmalloc(). In the following
> > > > execution, if the I/O APIC is used, this table is actually not used.
> > > > However, in that case, the allocated table is not freed, which can lead to
> > > > a memory leak bug.
> > >
> > > s/which can lead to/which is/
> > >
> > > There is no 'can'. It simply is a memory leak.
> > >
> > > > To fix this issue, this patch frees the allocated table if it is not used.
> > >
> > > To fix this issue, free the allocated table if it is not used.
> > >
> > > 'this patch' is completely redundant information and discouraged in
> > > Documentation/process/....
> > >
> >
> > Thanks for your suggestions, Thomas. I will revise the commit's message.
> >
> > Wenwen
> >
> > > Other than that:
> > >
> > > Acked-by: Thomas Gleixner <tglx@linutronix.de>
>
> You didn't add Thomas's Acked-by to your commit ...
>

I will add it and resubmit the patch.

Thanks,
Wenwen
diff mbox series

Patch

diff --git a/arch/x86/pci/irq.c b/arch/x86/pci/irq.c
index 52e5510..d3a73f9 100644
--- a/arch/x86/pci/irq.c
+++ b/arch/x86/pci/irq.c
@@ -1119,6 +1119,8 @@  static const struct dmi_system_id pciirq_dmi_table[] __initconst = {
 
 void __init pcibios_irq_init(void)
 {
+	struct irq_routing_table *rtable = NULL;
+
 	DBG(KERN_DEBUG "PCI: IRQ init\n");
 
 	if (raw_pci_ops == NULL)
@@ -1129,8 +1131,10 @@  void __init pcibios_irq_init(void)
 	pirq_table = pirq_find_routing_table();
 
 #ifdef CONFIG_PCI_BIOS
-	if (!pirq_table && (pci_probe & PCI_BIOS_IRQ_SCAN))
+	if (!pirq_table && (pci_probe & PCI_BIOS_IRQ_SCAN)) {
 		pirq_table = pcibios_get_irq_routing_table();
+		rtable = pirq_table;
+	}
 #endif
 	if (pirq_table) {
 		pirq_peer_trick();
@@ -1145,8 +1149,10 @@  void __init pcibios_irq_init(void)
 		 * If we're using the I/O APIC, avoid using the PCI IRQ
 		 * routing table
 		 */
-		if (io_apic_assign_pci_irqs)
+		if (io_apic_assign_pci_irqs) {
+			kfree(rtable);
 			pirq_table = NULL;
+		}
 	}
 
 	x86_init.pci.fixup_irqs();