| Message ID | 20250117090903.3329039-1-christian.bruel@foss.st.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: endpoint: pci-epf-test: Fix double free Oops | expand |
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>
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; } }
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(+)