public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH] arm64: vdso: fix AArch32 compat init allocation leaks
@ 2026-03-23 21:41 Osama Abdelkader
  2026-03-23 22:29 ` Andrew Morton
  2026-03-24  9:59 ` Will Deacon
  0 siblings, 2 replies; 6+ messages in thread
From: Osama Abdelkader @ 2026-03-23 21:41 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Kees Cook, Osama Abdelkader,
	Andrew Morton, Liam R. Howlett, Jeff Xu, linux-arm-kernel,
	linux-kernel

aarch32_alloc_vdso_pages() allocates the AA32 vdso pagelist, the compat
sigpage, then the kuser vectors page. If aarch32_alloc_sigpage() or
aarch32_alloc_kuser_vdso_page() fails, earlier allocations were not freed.

Unwind in reverse order: drop the sigpage when kuser setup fails, and
kfree the vdso pagelist when either later step fails (only when
CONFIG_COMPAT_VDSO allocated it).

Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
---
 arch/arm64/kernel/vdso.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index 592dd8668de4..9903bfdfd45e 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -236,9 +236,27 @@ static int __init aarch32_alloc_vdso_pages(void)
 
 	ret = aarch32_alloc_sigpage();
 	if (ret)
-		return ret;
+		goto free_vdso;
+
+	ret = aarch32_alloc_kuser_vdso_page();
+	if (ret)
+		goto free_sig;
+
+	return 0;
 
-	return aarch32_alloc_kuser_vdso_page();
+free_sig:
+	if (aarch32_sig_page) {
+		__free_page(aarch32_sig_page);
+		aarch32_sig_page = NULL;
+	}
+free_vdso:
+#ifdef CONFIG_COMPAT_VDSO
+	if (vdso_info[VDSO_ABI_AA32].cm && vdso_info[VDSO_ABI_AA32].cm->pages) {
+		kfree(vdso_info[VDSO_ABI_AA32].cm->pages);
+		vdso_info[VDSO_ABI_AA32].cm->pages = NULL;
+	}
+#endif
+	return ret;
 }
 arch_initcall(aarch32_alloc_vdso_pages);
 
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: vdso: fix AArch32 compat init allocation leaks
  2026-03-23 21:41 [PATCH] arm64: vdso: fix AArch32 compat init allocation leaks Osama Abdelkader
@ 2026-03-23 22:29 ` Andrew Morton
  2026-03-24  9:59 ` Will Deacon
  1 sibling, 0 replies; 6+ messages in thread
From: Andrew Morton @ 2026-03-23 22:29 UTC (permalink / raw)
  To: Osama Abdelkader
  Cc: Catalin Marinas, Will Deacon, Kees Cook, Liam R. Howlett, Jeff Xu,
	linux-arm-kernel, linux-kernel

On Mon, 23 Mar 2026 22:41:16 +0100 Osama Abdelkader <osama.abdelkader@gmail.com> wrote:

> aarch32_alloc_vdso_pages() allocates the AA32 vdso pagelist, the compat
> sigpage, then the kuser vectors page. If aarch32_alloc_sigpage() or
> aarch32_alloc_kuser_vdso_page() fails, earlier allocations were not freed.
> 
> Unwind in reverse order: drop the sigpage when kuser setup fails, and
> kfree the vdso pagelist when either later step fails (only when
> CONFIG_COMPAT_VDSO allocated it).

Thanks.  AI review has flagged a couple of possible issues:
	https://sashiko.dev/#/patchset/20260323214117.241216-1-osama.abdelkader@gmail.com


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: vdso: fix AArch32 compat init allocation leaks
  2026-03-23 21:41 [PATCH] arm64: vdso: fix AArch32 compat init allocation leaks Osama Abdelkader
  2026-03-23 22:29 ` Andrew Morton
@ 2026-03-24  9:59 ` Will Deacon
  2026-03-24 10:09   ` Thomas Weißschuh
  1 sibling, 1 reply; 6+ messages in thread
From: Will Deacon @ 2026-03-24  9:59 UTC (permalink / raw)
  To: Osama Abdelkader
  Cc: Catalin Marinas, Kees Cook, Andrew Morton, Liam R. Howlett,
	Jeff Xu, linux-arm-kernel, linux-kernel

On Mon, Mar 23, 2026 at 10:41:16PM +0100, Osama Abdelkader wrote:
> aarch32_alloc_vdso_pages() allocates the AA32 vdso pagelist, the compat
> sigpage, then the kuser vectors page. If aarch32_alloc_sigpage() or
> aarch32_alloc_kuser_vdso_page() fails, earlier allocations were not freed.

But why should they be freed? The vectors, sigpage and vdso are
independent from one another, so we can limp along with whatever we
managed to allocate. I'm not sure how far we'll get, mind, if single
page allocations are failing at initcall time...

Will


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: vdso: fix AArch32 compat init allocation leaks
  2026-03-24  9:59 ` Will Deacon
@ 2026-03-24 10:09   ` Thomas Weißschuh
  2026-03-24 10:14     ` Will Deacon
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Weißschuh @ 2026-03-24 10:09 UTC (permalink / raw)
  To: Will Deacon
  Cc: Osama Abdelkader, Catalin Marinas, Kees Cook, Andrew Morton,
	Liam R. Howlett, Jeff Xu, linux-arm-kernel, linux-kernel

On Tue, Mar 24, 2026 at 09:59:15AM +0000, Will Deacon wrote:
> On Mon, Mar 23, 2026 at 10:41:16PM +0100, Osama Abdelkader wrote:
> > aarch32_alloc_vdso_pages() allocates the AA32 vdso pagelist, the compat
> > sigpage, then the kuser vectors page. If aarch32_alloc_sigpage() or
> > aarch32_alloc_kuser_vdso_page() fails, earlier allocations were not freed.
> 
> But why should they be freed? The vectors, sigpage and vdso are
> independent from one another, so we can limp along with whatever we
> managed to allocate. I'm not sure how far we'll get, mind, if single
> page allocations are failing at initcall time...

In the core vDSO datastore we just panic() if the allocation fails.
(See tip/timers/vdso for the currentl implementation)
The same should work for the architecture-specific bits.

Also I am wondering again why the return values of initcalls are ignored.


Thomas


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: vdso: fix AArch32 compat init allocation leaks
  2026-03-24 10:09   ` Thomas Weißschuh
@ 2026-03-24 10:14     ` Will Deacon
  2026-03-24 16:13       ` Osama Abdelkader
  0 siblings, 1 reply; 6+ messages in thread
From: Will Deacon @ 2026-03-24 10:14 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Osama Abdelkader, Catalin Marinas, Kees Cook, Andrew Morton,
	Liam R. Howlett, Jeff Xu, linux-arm-kernel, linux-kernel

On Tue, Mar 24, 2026 at 11:09:12AM +0100, Thomas Weißschuh wrote:
> On Tue, Mar 24, 2026 at 09:59:15AM +0000, Will Deacon wrote:
> > On Mon, Mar 23, 2026 at 10:41:16PM +0100, Osama Abdelkader wrote:
> > > aarch32_alloc_vdso_pages() allocates the AA32 vdso pagelist, the compat
> > > sigpage, then the kuser vectors page. If aarch32_alloc_sigpage() or
> > > aarch32_alloc_kuser_vdso_page() fails, earlier allocations were not freed.
> > 
> > But why should they be freed? The vectors, sigpage and vdso are
> > independent from one another, so we can limp along with whatever we
> > managed to allocate. I'm not sure how far we'll get, mind, if single
> > page allocations are failing at initcall time...
> 
> In the core vDSO datastore we just panic() if the allocation fails.
> (See tip/timers/vdso for the currentl implementation)
> The same should work for the architecture-specific bits.

I think we should just leave the code as-is tbh unless there's an actual
issue here.

Will


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: vdso: fix AArch32 compat init allocation leaks
  2026-03-24 10:14     ` Will Deacon
@ 2026-03-24 16:13       ` Osama Abdelkader
  0 siblings, 0 replies; 6+ messages in thread
From: Osama Abdelkader @ 2026-03-24 16:13 UTC (permalink / raw)
  To: Will Deacon
  Cc: Thomas Weißschuh, Catalin Marinas, Kees Cook, Andrew Morton,
	Liam R. Howlett, Jeff Xu, linux-arm-kernel, linux-kernel

On Tue, Mar 24, 2026 at 10:14:38AM +0000, Will Deacon wrote:
> On Tue, Mar 24, 2026 at 11:09:12AM +0100, Thomas Weißschuh wrote:
> > On Tue, Mar 24, 2026 at 09:59:15AM +0000, Will Deacon wrote:
> > > On Mon, Mar 23, 2026 at 10:41:16PM +0100, Osama Abdelkader wrote:
> > > > aarch32_alloc_vdso_pages() allocates the AA32 vdso pagelist, the compat
> > > > sigpage, then the kuser vectors page. If aarch32_alloc_sigpage() or
> > > > aarch32_alloc_kuser_vdso_page() fails, earlier allocations were not freed.
> > > 
> > > But why should they be freed? The vectors, sigpage and vdso are
> > > independent from one another, so we can limp along with whatever we
> > > managed to allocate. I'm not sure how far we'll get, mind, if single
> > > page allocations are failing at initcall time...
> > 
> > In the core vDSO datastore we just panic() if the allocation fails.
> > (See tip/timers/vdso for the currentl implementation)
> > The same should work for the architecture-specific bits.
> 
> I think we should just leave the code as-is tbh unless there's an actual
> issue here.
> 
> Will

Thanks all for the review.

Regards,
Osama


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-03-24 16:14 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-23 21:41 [PATCH] arm64: vdso: fix AArch32 compat init allocation leaks Osama Abdelkader
2026-03-23 22:29 ` Andrew Morton
2026-03-24  9:59 ` Will Deacon
2026-03-24 10:09   ` Thomas Weißschuh
2026-03-24 10:14     ` Will Deacon
2026-03-24 16:13       ` Osama Abdelkader

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox