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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Z4ozsi_Le-Nk_25N@ryzen>
Date: Fri, 17 Jan 2025 11:40:50 +0100
From: Niklas Cassel <cassel@...nel.org>
To: Christian Bruel <christian.bruel@...s.st.com>
Cc: manivannan.sadhasivam@...aro.org, kw@...ux.com, kishon@...nel.org,
	bhelgaas@...gle.com, Frank.Li@....com, dlemoal@...nel.org,
	fabrice.gasnier@...s.st.com, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] PCI: endpoint: pci-epf-test: Fix double free Oops

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@...s.st.com>
> ---

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