[PATCH] arm64: vdso: fix AArch32 compat init allocation leaks

Osama Abdelkader posted 1 patch 1 week, 5 days ago
arch/arm64/kernel/vdso.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
[PATCH] arm64: vdso: fix AArch32 compat init allocation leaks
Posted by Osama Abdelkader 1 week, 5 days ago
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
Re: [PATCH] arm64: vdso: fix AArch32 compat init allocation leaks
Posted by Will Deacon 1 week, 5 days ago
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
Re: [PATCH] arm64: vdso: fix AArch32 compat init allocation leaks
Posted by Thomas Weißschuh 1 week, 5 days ago
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
Re: [PATCH] arm64: vdso: fix AArch32 compat init allocation leaks
Posted by Will Deacon 1 week, 5 days ago
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
Re: [PATCH] arm64: vdso: fix AArch32 compat init allocation leaks
Posted by Osama Abdelkader 1 week, 4 days ago
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
Re: [PATCH] arm64: vdso: fix AArch32 compat init allocation leaks
Posted by Andrew Morton 1 week, 5 days ago
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