[PATCH v2 0/2] KVM: arm64: KVM: arm64: Fix potential leak in hyp_trace_buffer_alloc_bpages_backing

[PATCH v2 0/2] KVM: arm64: KVM: arm64: Fix potential leak in hyp_trace_buffer_alloc_bpages_backing

[Vincent Donnefort]

This series provides two fixes for the nVHE/pKVM hyp trace remote buffer
allocation.

Changelog:

v2:
  - Added patch: "KVM: arm64: Fix hyp_trace_desc allocation size in
    hyp_trace_load()"
  - Collected Reviewed-by and Tested-by tags from Fuad Tabba for the
    leak fix
  - Do not set bpages_backing_* on __map_hyp() failure (Fuad)

v1: https://lore.kernel.org/all/20260707165029.3410701-1-vdonnefort@google.com/

Vincent Donnefort (2):
  KVM: arm64: Fix potential leak in
    hyp_trace_buffer_alloc_bpages_backing
  KVM: arm64: Fix hyp_trace_desc allocation size in hyp_trace_load()

 arch/arm64/kvm/hyp_trace.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)


base-commit: 8cdeaa50eae8dad34885515f62559ee83e7e8dda
-- 
2.55.0.795.g602f6c329a-goog

[PATCH v2 1/2] KVM: arm64: Fix potential leak in hyp_trace_buffer_alloc_bpages_backing

[Vincent Donnefort]

In the very unlikely event of a failure in __map_hyp, the allocated
backing pages are leaked in hyp_trace_buffer_alloc_bpages_backing(). Fix
this by freeing the pages on error.

Fixes: 3aed038aac8d ("KVM: arm64: Add trace remote for the nVHE/pKVM hyp")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Reviewed-by: Fuad Tabba <fuad.tabba@linux.dev>
Tested-by: Fuad Tabba <fuad.tabba@linux.dev>
Signed-off-by: Vincent Donnefort <vdonnefort@google.com>

diff --git a/arch/arm64/kvm/hyp_trace.c b/arch/arm64/kvm/hyp_trace.c
index 2411b4c32932..a7237aca6898 100644
--- a/arch/arm64/kvm/hyp_trace.c
+++ b/arch/arm64/kvm/hyp_trace.c
@@ -160,6 +160,7 @@ static int hyp_trace_buffer_alloc_bpages_backing(struct hyp_trace_buffer *trace_
 	int nr_bpages = (PAGE_ALIGN(size) / PAGE_SIZE) + 1;
 	size_t backing_size;
 	void *start;
+	int ret;
 
 	backing_size = PAGE_ALIGN(sizeof(struct simple_buffer_page) * nr_bpages *
 				  num_possible_cpus());
@@ -168,10 +169,16 @@ static int hyp_trace_buffer_alloc_bpages_backing(struct hyp_trace_buffer *trace_
 	if (!start)
 		return -ENOMEM;
 
+	ret = __map_hyp(start, backing_size);
+	if (ret) {
+		free_pages_exact(start, backing_size);
+		return ret;
+	}
+
 	trace_buffer->desc->bpages_backing_start = (unsigned long)start;
 	trace_buffer->desc->bpages_backing_size = backing_size;
 
-	return __map_hyp(start, backing_size);
+	return ret;
 }
 
 static void hyp_trace_buffer_free_bpages_backing(struct hyp_trace_buffer *trace_buffer)
-- 
2.55.0.795.g602f6c329a-goog

[PATCH v2 2/2] KVM: arm64: Fix hyp_trace_desc allocation size in hyp_trace_load()

[Vincent Donnefort]

The footprint calculated for struct hyp_trace_desc sizes only
trace_buffer_desc and do not take into account the other fields. It
worked so far thanks to the follow-up PAGE_ALIGN().

Fix the descriptor size and while at it, enforce an overflow check after
PAGE_ALIGN().

Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: 3aed038aac8d ("KVM: arm64: Add trace remote for the nVHE/pKVM hyp")
Signed-off-by: Vincent Donnefort <vdonnefort@google.com>

diff --git a/arch/arm64/kvm/hyp_trace.c b/arch/arm64/kvm/hyp_trace.c
index a7237aca6898..22437c5e1e7b 100644
--- a/arch/arm64/kvm/hyp_trace.c
+++ b/arch/arm64/kvm/hyp_trace.c
@@ -235,18 +235,22 @@ static int hyp_trace_buffer_share_hyp(struct hyp_trace_buffer *trace_buffer)
 static struct trace_buffer_desc *hyp_trace_load(unsigned long size, void *priv)
 {
 	struct hyp_trace_buffer *trace_buffer = priv;
+	size_t desc_size, tb_desc_size;
 	struct hyp_trace_desc *desc;
-	size_t desc_size;
 	int ret;
 
 	if (WARN_ON(trace_buffer->desc))
 		return ERR_PTR(-EINVAL);
 
-	desc_size = trace_buffer_desc_size(size, num_possible_cpus());
+	tb_desc_size = trace_buffer_desc_size(size, num_possible_cpus());
+	desc_size = size_add(tb_desc_size, offsetof(struct hyp_trace_desc, trace_buffer_desc));
 	if (desc_size == SIZE_MAX)
 		return ERR_PTR(-E2BIG);
 
 	desc_size = PAGE_ALIGN(desc_size);
+	if (!desc_size)
+		return ERR_PTR(-E2BIG);
+
 	desc = (struct hyp_trace_desc *)alloc_pages_exact(desc_size, GFP_KERNEL);
 	if (!desc)
 		return ERR_PTR(-ENOMEM);
@@ -262,7 +266,7 @@ static struct trace_buffer_desc *hyp_trace_load(unsigned long size, void *priv)
 	if (ret)
 		goto err_free_desc;
 
-	ret = trace_remote_alloc_buffer(&desc->trace_buffer_desc, desc_size, size,
+	ret = trace_remote_alloc_buffer(&desc->trace_buffer_desc, tb_desc_size, size,
 					cpu_possible_mask);
 	if (ret)
 		goto err_free_backing;
-- 
2.55.0.795.g602f6c329a-goog

Re: [PATCH v2 2/2] KVM: arm64: Fix hyp_trace_desc allocation size in hyp_trace_load()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Incorrect bounds calculation due to pointer arithmetic on struct pointers leads to bounds check bypass and critical TOCTOU vulnerability.
--

--- Patch [2]: [PATCH v2 2/2] KVM: arm64: Fix hyp_trace_desc allocation size in hyp_trace_load() ---
commit 6dde5b05614b22025af630cb338d61676ac3c7d8
Author: Vincent Donnefort <vdonnefort@google.com>

KVM: arm64: Fix hyp_trace_desc allocation size in hyp_trace_load()

The footprint calculated for struct hyp_trace_desc sizes only
trace_buffer_desc and do not take into account the other fields. It
worked so far thanks to the follow-up PAGE_ALIGN().

> diff --git a/arch/arm64/kvm/hyp_trace.c b/arch/arm64/kvm/hyp_trace.c
> index a7237aca68989..22437c5e1e7bc 100644
> --- a/arch/arm64/kvm/hyp_trace.c
> +++ b/arch/arm64/kvm/hyp_trace.c
[ ... ]
> @@ -262,7 +266,7 @@ static struct trace_buffer_desc *hyp_trace_load(unsigned long size, void *priv)
>  	if (ret)
>  		goto err_free_desc;
>  
> -	ret = trace_remote_alloc_buffer(&desc->trace_buffer_desc, desc_size, size,
> +	ret = trace_remote_alloc_buffer(&desc->trace_buffer_desc, tb_desc_size, size,
>  					cpu_possible_mask);

[Severity: Critical]
This is a pre-existing issue, but does the bounds check in
trace_remote_alloc_buffer() calculate desc_end correctly?

In kernel/trace/trace_remote.c:trace_remote_alloc_buffer(), desc_end is
calculated as:

	void *desc_end = desc + desc_size;

Since desc is a typed pointer (struct trace_buffer_desc *), this pointer
arithmetic scales desc_size by sizeof(struct trace_buffer_desc), inflating
the bounds check and bypassing the intended protection.

A similar issue appears to exist in arch/arm64/kvm/hyp/nvhe/trace.c inside
hyp_trace_desc_is_valid(), where pointer arithmetic scales desc_size by
sizeof(struct hyp_trace_desc). This could potentially allow EL2 to read
un-donated host memory, opening a TOCTOU vulnerability if the host modifies
bounds after the validation check.

Should these pointers be cast to a byte-sized type like (void *) or (u8 *)
before adding the size?

>  	if (ret)
>  		goto err_free_backing;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710114819.2689386-1-vdonnefort@google.com?part=2

Re: [PATCH v2 2/2] KVM: arm64: Fix hyp_trace_desc allocation size in hyp_trace_load()

[Vincent Donnefort]

On Fri, Jul 10, 2026 at 12:02:54PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [Critical] Incorrect bounds calculation due to pointer arithmetic on struct pointers leads to bounds check bypass and critical TOCTOU vulnerability.
> --
> 
> --- Patch [2]: [PATCH v2 2/2] KVM: arm64: Fix hyp_trace_desc allocation size in hyp_trace_load() ---
> commit 6dde5b05614b22025af630cb338d61676ac3c7d8
> Author: Vincent Donnefort <vdonnefort@google.com>
> 
> KVM: arm64: Fix hyp_trace_desc allocation size in hyp_trace_load()
> 
> The footprint calculated for struct hyp_trace_desc sizes only
> trace_buffer_desc and do not take into account the other fields. It
> worked so far thanks to the follow-up PAGE_ALIGN().
> 
> > diff --git a/arch/arm64/kvm/hyp_trace.c b/arch/arm64/kvm/hyp_trace.c
> > index a7237aca68989..22437c5e1e7bc 100644
> > --- a/arch/arm64/kvm/hyp_trace.c
> > +++ b/arch/arm64/kvm/hyp_trace.c
> [ ... ]
> > @@ -262,7 +266,7 @@ static struct trace_buffer_desc *hyp_trace_load(unsigned long size, void *priv)
> >  	if (ret)
> >  		goto err_free_desc;
> >  
> > -	ret = trace_remote_alloc_buffer(&desc->trace_buffer_desc, desc_size, size,
> > +	ret = trace_remote_alloc_buffer(&desc->trace_buffer_desc, tb_desc_size, size,
> >  					cpu_possible_mask);
> 
> [Severity: Critical]
> This is a pre-existing issue, but does the bounds check in
> trace_remote_alloc_buffer() calculate desc_end correctly?
> 
> In kernel/trace/trace_remote.c:trace_remote_alloc_buffer(), desc_end is
> calculated as:
> 
> 	void *desc_end = desc + desc_size;
> 
> Since desc is a typed pointer (struct trace_buffer_desc *), this pointer
> arithmetic scales desc_size by sizeof(struct trace_buffer_desc), inflating
> the bounds check and bypassing the intended protection.

This is already part of this fix: https://lore.kernel.org/all/20260709160017.1729517-3-vdonnefort@google.com/

> 
> A similar issue appears to exist in arch/arm64/kvm/hyp/nvhe/trace.c inside
> hyp_trace_desc_is_valid(), where pointer arithmetic scales desc_size by
> sizeof(struct hyp_trace_desc). This could potentially allow EL2 to read
> un-donated host memory, opening a TOCTOU vulnerability if the host modifies
> bounds after the validation check.

I don't see any issue in hyp_trace_desc_is_valid() though. 

> 
> Should these pointers be cast to a byte-sized type like (void *) or (u8 *)
> before adding the size?
> 
> >  	if (ret)
> >  		goto err_free_backing;
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260710114819.2689386-1-vdonnefort@google.com?part=2

Re: [PATCH v2 2/2] KVM: arm64: Fix hyp_trace_desc allocation size in hyp_trace_load()

[Fuad Tabba]

On Fri, 10 Jul 2026 at 12:48, Vincent Donnefort <vdonnefort@google.com> wrote:
>
> The footprint calculated for struct hyp_trace_desc sizes only
> trace_buffer_desc and do not take into account the other fields. It
> worked so far thanks to the follow-up PAGE_ALIGN().
>
> Fix the descriptor size and while at it, enforce an overflow check after
> PAGE_ALIGN().
>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Fixes: 3aed038aac8d ("KVM: arm64: Add trace remote for the nVHE/pKVM hyp")
> Signed-off-by: Vincent Donnefort <vdonnefort@google.com>

Applied both patches on v7.2-rc2 and confirmed
arch/arm64/kvm/hyp_trace.c compiles (CONFIG_NVHE_EL2_TRACING=y).
Booted protected and non-protected VMs. To exercise hyp_trace_load()
directly I enabled the hypervisor trace remote via tracefs
(remotes/hypervisor/tracing_on) at the default 7KiB and reloaded at
512KiB, so the reworked desc_size/tb_desc_size split and the
alloc/map/share/__tracing_load path all ran fine.

Reviewed-by: Fuad Tabba <fuad.tabba@linux.dev>
Tested-by: Fuad Tabba < fuad.tabba@linux.dev>

Cheers,
/fuad

>
> diff --git a/arch/arm64/kvm/hyp_trace.c b/arch/arm64/kvm/hyp_trace.c
> index a7237aca6898..22437c5e1e7b 100644
> --- a/arch/arm64/kvm/hyp_trace.c
> +++ b/arch/arm64/kvm/hyp_trace.c
> @@ -235,18 +235,22 @@ static int hyp_trace_buffer_share_hyp(struct hyp_trace_buffer *trace_buffer)
>  static struct trace_buffer_desc *hyp_trace_load(unsigned long size, void *priv)
>  {
>         struct hyp_trace_buffer *trace_buffer = priv;
> +       size_t desc_size, tb_desc_size;
>         struct hyp_trace_desc *desc;
> -       size_t desc_size;
>         int ret;
>
>         if (WARN_ON(trace_buffer->desc))
>                 return ERR_PTR(-EINVAL);
>
> -       desc_size = trace_buffer_desc_size(size, num_possible_cpus());
> +       tb_desc_size = trace_buffer_desc_size(size, num_possible_cpus());
> +       desc_size = size_add(tb_desc_size, offsetof(struct hyp_trace_desc, trace_buffer_desc));
>         if (desc_size == SIZE_MAX)
>                 return ERR_PTR(-E2BIG);
>
>         desc_size = PAGE_ALIGN(desc_size);
> +       if (!desc_size)
> +               return ERR_PTR(-E2BIG);
> +
>         desc = (struct hyp_trace_desc *)alloc_pages_exact(desc_size, GFP_KERNEL);
>         if (!desc)
>                 return ERR_PTR(-ENOMEM);
> @@ -262,7 +266,7 @@ static struct trace_buffer_desc *hyp_trace_load(unsigned long size, void *priv)
>         if (ret)
>                 goto err_free_desc;
>
> -       ret = trace_remote_alloc_buffer(&desc->trace_buffer_desc, desc_size, size,
> +       ret = trace_remote_alloc_buffer(&desc->trace_buffer_desc, tb_desc_size, size,
>                                         cpu_possible_mask);
>         if (ret)
>                 goto err_free_backing;
> --
> 2.55.0.795.g602f6c329a-goog
>