* [PATCH] PCI: endpoint: pci-epf-test: Fix double free Oops
@ 2025-01-17 9:09 Christian Bruel
2025-01-17 10:40 ` Niklas Cassel
2025-01-24 7:48 ` Manivannan Sadhasivam
0 siblings, 2 replies; 3+ messages in thread
From: Christian Bruel @ 2025-01-17 9:09 UTC (permalink / raw)
To: manivannan.sadhasivam, kw, kishon, bhelgaas, cassel, Frank.Li,
dlemoal, fabrice.gasnier
Cc: linux-pci, linux-kernel, Christian Bruel
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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] PCI: endpoint: pci-epf-test: Fix double free Oops
2025-01-17 9:09 [PATCH] PCI: endpoint: pci-epf-test: Fix double free Oops Christian Bruel
@ 2025-01-17 10:40 ` Niklas Cassel
2025-01-24 7:48 ` Manivannan Sadhasivam
1 sibling, 0 replies; 3+ messages in thread
From: Niklas Cassel @ 2025-01-17 10:40 UTC (permalink / raw)
To: Christian Bruel
Cc: manivannan.sadhasivam, kw, kishon, bhelgaas, Frank.Li, dlemoal,
fabrice.gasnier, linux-pci, linux-kernel
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>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] PCI: endpoint: pci-epf-test: Fix double free Oops
2025-01-17 9:09 [PATCH] PCI: endpoint: pci-epf-test: Fix double free Oops Christian Bruel
2025-01-17 10:40 ` Niklas Cassel
@ 2025-01-24 7:48 ` Manivannan Sadhasivam
1 sibling, 0 replies; 3+ messages in thread
From: Manivannan Sadhasivam @ 2025-01-24 7:48 UTC (permalink / raw)
To: Christian Bruel
Cc: manivannan.sadhasivam, kw, kishon, bhelgaas, cassel, Frank.Li,
dlemoal, fabrice.gasnier, linux-pci, linux-kernel
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
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-01-24 7:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-17 9:09 [PATCH] PCI: endpoint: pci-epf-test: Fix double free Oops Christian Bruel
2025-01-17 10:40 ` Niklas Cassel
2025-01-24 7:48 ` Manivannan Sadhasivam
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox