All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vincent Donnefort <vdonnefort@google.com>
To: sashiko-reviews@lists.linux.dev
Cc: kvmarm@lists.linux.dev, Oliver Upton <oupton@kernel.org>,
	Marc Zyngier <maz@kernel.org>
Subject: Re: [PATCH v2 2/2] KVM: arm64: Fix hyp_trace_desc allocation size in hyp_trace_load()
Date: Fri, 10 Jul 2026 14:19:08 +0100	[thread overview]
Message-ID: <alDxTK1uPuecd2dY@google.com> (raw)
In-Reply-To: <20260710120255.883B01F000E9@smtp.kernel.org>

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

  reply	other threads:[~2026-07-10 13:19 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-10 11:48 [PATCH v2 0/2] KVM: arm64: KVM: arm64: Fix potential leak in hyp_trace_buffer_alloc_bpages_backing Vincent Donnefort
2026-07-10 11:48 ` [PATCH v2 1/2] " Vincent Donnefort
2026-07-10 11:48 ` [PATCH v2 2/2] KVM: arm64: Fix hyp_trace_desc allocation size in hyp_trace_load() Vincent Donnefort
2026-07-10 12:02   ` sashiko-bot
2026-07-10 13:19     ` Vincent Donnefort [this message]
2026-07-12 15:56   ` Fuad Tabba

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alDxTK1uPuecd2dY@google.com \
    --to=vdonnefort@google.com \
    --cc=kvmarm@lists.linux.dev \
    --cc=maz@kernel.org \
    --cc=oupton@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.