1. Patchew
  2. linux
  3. View series

[PATCH] PCI: endpoint: pci-epf-test: Fix double free Oops

Christian Bruel posted 1 patch 1 year ago
There is a newer version of this series
drivers/pci/endpoint/functions/pci-epf-test.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH] PCI: endpoint: pci-epf-test: Fix double free Oops
Posted by Christian Bruel 1 year ago 
Fixes an oops found while testing the stm32_pcie ep driver with handling
of PERST# deassertion:

[   92.154549] ------------[ cut here ]------------
[   92.159093] Trying to vunmap() nonexistent vm area (0000000031e0f06f)
...
[   92.288763]  vunmap+0x58/0x60 (P)
[   92.292096]  dma_direct_free+0x88/0x18c
[   92.295932]  dma_free_attrs+0x84/0xf8
[   92.299664]  pci_epf_free_space+0x48/0x78
[   92.303698]  pci_epf_test_epc_init+0x184/0x3c0 [pci_epf_test]
[   92.309446]  pci_epc_init_notify+0x70/0xb4
[   92.313578]  stm32_pcie_ep_perst_irq_thread+0xf8/0x24c
...

During EP initialization, pci_epf_test_alloc_space allocates all BARs,
which are further freed if epc_set_bar fails (for instance, due to
no free inbound window).

However, when pci_epc_set_bar fails, the error path:
     pci_epc_set_bar -> pci_epf_free_space
does not reset epf_test->reg[bar].

Then, if the host reboots, PERST# deassertion restarts the BAR allocation
sequence with the same allocation failure (no free inbound window).

So, two subsequent calls to the sequence:

  if (!epf_test->reg[bar])
      continue;

  ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no, &epf->bar[bar]);
  if (ret) {
      pci_epf_free_space(epf, epf_test->reg[bar], bar, PRIMARY_INTERFACE);
  }

create a double free situation since epf_test->reg[bar] was deallocated
and is still non-NULL.

This patch makes pci_epf_alloc_space/pci_epf_free_space symmetric
by resetting epf_test->reg[bar] when memory is deallocated.

Signed-off-by: Christian Bruel <christian.bruel@foss.st.com>
---
 drivers/pci/endpoint/functions/pci-epf-test.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index ffb534a8e50a..b29e938ee16a 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -718,6 +718,7 @@ static int pci_epf_test_set_bar(struct pci_epf *epf)
 		if (ret) {
 			pci_epf_free_space(epf, epf_test->reg[bar], bar,
 					   PRIMARY_INTERFACE);
+			epf_test->reg[bar] = NULL;
 			dev_err(dev, "Failed to set BAR%d\n", bar);
 			if (bar == test_reg_bar)
 				return ret;
@@ -909,6 +910,7 @@ static void pci_epf_test_free_space(struct pci_epf *epf)
 
 		pci_epf_free_space(epf, epf_test->reg[bar], bar,
 				   PRIMARY_INTERFACE);
+		epf_test->reg[bar] = NULL;
 	}
 }
 
-- 
2.34.1
Re: [PATCH] PCI: endpoint: pci-epf-test: Fix double free Oops
Posted by Manivannan Sadhasivam 1 year ago 
On Fri, Jan 17, 2025 at 10:09:03AM +0100, Christian Bruel wrote:
> Fixes an oops found while testing the stm32_pcie ep driver with handling
> of PERST# deassertion:
> 
> [   92.154549] ------------[ cut here ]------------
> [   92.159093] Trying to vunmap() nonexistent vm area (0000000031e0f06f)
> ...
> [   92.288763]  vunmap+0x58/0x60 (P)
> [   92.292096]  dma_direct_free+0x88/0x18c
> [   92.295932]  dma_free_attrs+0x84/0xf8
> [   92.299664]  pci_epf_free_space+0x48/0x78
> [   92.303698]  pci_epf_test_epc_init+0x184/0x3c0 [pci_epf_test]
> [   92.309446]  pci_epc_init_notify+0x70/0xb4
> [   92.313578]  stm32_pcie_ep_perst_irq_thread+0xf8/0x24c
> ...
> 
> During EP initialization, pci_epf_test_alloc_space allocates all BARs,
> which are further freed if epc_set_bar fails (for instance, due to
> no free inbound window).
> 
> However, when pci_epc_set_bar fails, the error path:
>      pci_epc_set_bar -> pci_epf_free_space
> does not reset epf_test->reg[bar].
> 
> Then, if the host reboots, PERST# deassertion restarts the BAR allocation
> sequence with the same allocation failure (no free inbound window).
> 
> So, two subsequent calls to the sequence:
> 
>   if (!epf_test->reg[bar])
>       continue;
> 
>   ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no, &epf->bar[bar]);
>   if (ret) {
>       pci_epf_free_space(epf, epf_test->reg[bar], bar, PRIMARY_INTERFACE);
>   }
> 
> create a double free situation since epf_test->reg[bar] was deallocated
> and is still non-NULL.
> 
> This patch makes pci_epf_alloc_space/pci_epf_free_space symmetric
> by resetting epf_test->reg[bar] when memory is deallocated.
> 
> Signed-off-by: Christian Bruel <christian.bruel@foss.st.com>

With comments from Niklas addressed,

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

- Mani

> ---
>  drivers/pci/endpoint/functions/pci-epf-test.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index ffb534a8e50a..b29e938ee16a 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -718,6 +718,7 @@ static int pci_epf_test_set_bar(struct pci_epf *epf)
>  		if (ret) {
>  			pci_epf_free_space(epf, epf_test->reg[bar], bar,
>  					   PRIMARY_INTERFACE);
> +			epf_test->reg[bar] = NULL;
>  			dev_err(dev, "Failed to set BAR%d\n", bar);
>  			if (bar == test_reg_bar)
>  				return ret;
> @@ -909,6 +910,7 @@ static void pci_epf_test_free_space(struct pci_epf *epf)
>  
>  		pci_epf_free_space(epf, epf_test->reg[bar], bar,
>  				   PRIMARY_INTERFACE);
> +		epf_test->reg[bar] = NULL;
>  	}
>  }
>  
> -- 
> 2.34.1
> 

-- 
மணிவண்ணன் சதாசிவம்
Re: [PATCH] PCI: endpoint: pci-epf-test: Fix double free Oops
Posted by Niklas Cassel 1 year ago 
On Fri, Jan 17, 2025 at 10:09:03AM +0100, Christian Bruel wrote:
> Fixes an oops found while testing the stm32_pcie ep driver with handling
> of PERST# deassertion:
> 
> [   92.154549] ------------[ cut here ]------------
> [   92.159093] Trying to vunmap() nonexistent vm area (0000000031e0f06f)
> ...
> [   92.288763]  vunmap+0x58/0x60 (P)
> [   92.292096]  dma_direct_free+0x88/0x18c
> [   92.295932]  dma_free_attrs+0x84/0xf8
> [   92.299664]  pci_epf_free_space+0x48/0x78
> [   92.303698]  pci_epf_test_epc_init+0x184/0x3c0 [pci_epf_test]
> [   92.309446]  pci_epc_init_notify+0x70/0xb4
> [   92.313578]  stm32_pcie_ep_perst_irq_thread+0xf8/0x24c
> ...

Personally, I would omit the stack trace.


> 
> During EP initialization, pci_epf_test_alloc_space allocates all BARs,
> which are further freed if epc_set_bar fails (for instance, due to
> no free inbound window).
> 
> However, when pci_epc_set_bar fails, the error path:
>      pci_epc_set_bar -> pci_epf_free_space
> does not reset epf_test->reg[bar].
> 
> Then, if the host reboots, PERST# deassertion restarts the BAR allocation
> sequence with the same allocation failure (no free inbound window).
> 
> So, two subsequent calls to the sequence:
> 
>   if (!epf_test->reg[bar])
>       continue;
> 
>   ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no, &epf->bar[bar]);
>   if (ret) {
>       pci_epf_free_space(epf, epf_test->reg[bar], bar, PRIMARY_INTERFACE);
>   }

Personally, I would omit the copy pasted code.


> 
> create a double free situation since epf_test->reg[bar] was deallocated
> and is still non-NULL.
> 
> This patch makes pci_epf_alloc_space/pci_epf_free_space symmetric
> by resetting epf_test->reg[bar] when memory is deallocated.
> 
> Signed-off-by: Christian Bruel <christian.bruel@foss.st.com>
> ---

Reviewed-by: Niklas Cassel <cassel@kernel.org>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.