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

[sashiko-bot@kernel.org]

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