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

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

[Wenwen Wang]

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(-)

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();
-- 
2.7.4

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

[Thomas Gleixner]

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>

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

[Wenwen Wang]

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>

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

[Ingo Molnar]

* 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

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

[Wenwen Wang]

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