Re: [PATCH v1 2/2] KVM: arm64: Fix bpages backing allocation size in hyp_trace

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] tracing: Integer overflow in trace buffer size calculation bypasses capacity check
--

commit 652836a529b433a8c0e0cb4f4308fa9b9556763d
Author: Vincent Donnefort <vdonnefort@google.com>

KVM: arm64: Fix bpages backing allocation size in hyp_trace

This commit updates the trace buffer backing allocation in hyp_trace to
calculate the required number of pages using
__calc_nr_pages_ring_buffer_desc(). This appropriately accounts for the
buffer page header and relies on trace_buffer_desc_size() for validation.

> diff --git a/arch/arm64/kvm/hyp_trace.c b/arch/arm64/kvm/hyp_trace.c
> index 26fc13c164589..b9404496f9775 100644
> --- a/arch/arm64/kvm/hyp_trace.c
> +++ b/arch/arm64/kvm/hyp_trace.c
[ ... ]
> @@ -157,7 +157,7 @@ static int hyp_trace_buffer_alloc_bpages_backing(struct hyp_trace_buffer *trace_
>  		return 0;
>  	}
>  
> -	nr_bpages = (PAGE_ALIGN(size) / PAGE_SIZE) + 1;
> +	nr_bpages = __calc_nr_pages_ring_buffer_desc(size);

[Severity: Low]
Does this calculation overflow if size is near SIZE_MAX?

Looking at __calc_nr_pages_ring_buffer_desc() in include/linux/ring_buffer.h:

static inline unsigned long __calc_nr_pages_ring_buffer_desc(size_t size)
{
        /* Takes into account the reader page */
        return max(DIV_ROUND_UP(size, PAGE_SIZE - BUF_PAGE_HDR_SIZE), 2UL) + 1;
}

If size is extremely large, the DIV_ROUND_UP macro wraps around, causing the
function to erroneously return a very small page count (e.g. 0 wrapping to 3
pages).

The commit message notes there is no need to validate nr_bpages again because
it was validated by trace_buffer_desc_size(), but could this overflow bypass
that check?

Looking at trace_buffer_desc_size() in include/linux/ring_buffer.h:

        /* Capped by ring_buffer_desc::nr_page_va */
        if (nr_pages > UINT_MAX)
                return SIZE_MAX;

Because the wrapped nr_pages is small, this capacity check would be
circumvented, bypassing the intended -E2BIG rejection. Downstream consumers
like hyp_trace_buffer_alloc_bpages_backing(), trace_remote_alloc_buffer(),
and __tracing_load() would then operate safely on this truncated page count.
This results in a silent failure where the kernel allocates a tiny functional
trace buffer instead of returning an error for the impossibly large
allocation.

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