Re: [PATCH] KVM: arm64: Fix potential leak in hyp_trace_buffer_alloc_bpages_backing

[Vincent Donnefort <vdonnefort@google.com>]

On Tue, Jul 07, 2026 at 05:02:44PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] trace_remote_alloc_buffer leaks page allocations on the error path if a per-CPU ring buffer allocation fails.
> --
> 
> --- Patch [1]: [PATCH] KVM: arm64: Fix potential leak in hyp_trace_buffer_alloc_bpages_backing ---
> commit 21f10293a05c7439f7a1d1be691e92ef2d36331a
> Author: Vincent Donnefort <vdonnefort@google.com>
> 
> KVM: arm64: Fix potential leak in hyp_trace_buffer_alloc_bpages_backing
> 
> 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>
> 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 2411b4c32932c..5fbbfd40376f7 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());
> > @@ -171,7 +172,11 @@ static int hyp_trace_buffer_alloc_bpages_backing(struct hyp_trace_buffer *trace_
> >  	trace_buffer->desc->bpages_backing_start = (unsigned long)start;
> >  	trace_buffer->desc->bpages_backing_size = backing_size;
> >  
> > -	return __map_hyp(start, backing_size);
> > +	ret = __map_hyp(start, backing_size);
> > +	if (ret)
> > +		free_pages_exact(start, backing_size);
> > +
> > +	return ret;
> >  }
> 
> [Severity: High]
> This is a pre-existing issue, but since this patch fixes a memory leak in the 
> trace buffer allocation, I noticed a potential leak in a related error path.
> 
> In kernel/trace/trace_remote.c:trace_remote_alloc_buffer(), if an allocation
> fails, the code jumps to the err label:
> 
> 	rb_desc->meta_va = (unsigned long)__get_free_page(GFP_KERNEL);
> 	if (!rb_desc->meta_va)
> 		goto err;
> 
> 	for (id = 0; id < nr_pages; id++) {
> 		rb_desc->page_va[id] = (unsigned long)__get_free_page(GFP_KERNEL);
> 		if (!rb_desc->page_va[id])
> 			goto err;
> 
> 		rb_desc->nr_page_va++;
> 	}
> 	desc->nr_cpus++;
> 
> Because desc->nr_cpus is only incremented after all pages for the current CPU
> are allocated, any successfully allocated pages for the failing CPU are left
> out of the count.
> 
> When the error path calls trace_remote_free_buffer(), it iterates based on
> desc->nr_cpus:
> 
> 	for_each_ring_buffer_desc(rb_desc, cpu, desc) {
> 		unsigned int id;
> 
> 		free_page(rb_desc->meta_va);
> 
> 		for (id = 0; id < rb_desc->nr_page_va; id++)
> 			free_page(rb_desc->page_va[id]);
> 	}
> 
> Does this skip the partially initialized ring buffer for the failing CPU and 
> leak its allocated pages?

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

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