Re: [PATCH v4 4/5] KVM: arm64: Fix bounds checking in do_ffa_mem_reclaim()

[Marc Zyngier <maz@kernel.org>]

On Thu, 21 May 2026 11:43:58 +0100,
Mostafa Saleh <smostafa@google.com> wrote:
> 
> On Thu, May 21, 2026 at 11:30 AM Mostafa Saleh <smostafa@google.com> wrote:
> >
> > Hi Marc,
> >
> > On Thu, May 21, 2026 at 09:28:46AM +0100, Marc Zyngier wrote:
> > > On Wed, 20 May 2026 21:49:47 +0100,
> > > Mostafa Saleh <smostafa@google.com> wrote:
> > > >
> > > > Sashiko (locally) reports out of bound write possiblity if SPMD
> > > > returns an invalid data.
> > > >
> > > > While SPMD is considered trusted, pKVM does some basic checks,
> > > > for offset to be less than or equal len.
> > > >
> > > > However, that is incorrect as even if the offset is smaller than
> > > > len pKVM can still access out of bound memory in the next
> > > > ffa_host_unshare_ranges().
> > > >
> > > > Split this check into 2:
> > > > 1- Check that the fixed portion of the descriptor fits.
> > > > 2- After getting reg, check the variable array size addr_range_cnt
> > > >    fits.
> > > >
> > > > Signed-off-by: Mostafa Saleh <smostafa@google.com>
> > > > ---
> > > >  arch/arm64/kvm/hyp/nvhe/ffa.c | 7 ++++++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
> > > > index 1af722771178..e6aa2bfa63b1 100644
> > > > --- a/arch/arm64/kvm/hyp/nvhe/ffa.c
> > > > +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
> > > > @@ -607,7 +607,7 @@ static void do_ffa_mem_reclaim(struct arm_smccc_1_2_regs *res,
> > > >      * check that we end up with something that doesn't look _completely_
> > > >      * bogus.
> > > >      */
> > > > -   if (WARN_ON(offset > len ||
> > > > +   if (WARN_ON(offset + CONSTITUENTS_OFFSET(0) > len ||
> > > >                 fraglen > KVM_FFA_MBOX_NR_PAGES * PAGE_SIZE)) {
> > >
> > > Do you really want to keep this a WARN_ON(), given that this results
> > > in a panic in most pKVM configurations?
> >
> > Which kind of configuration will that check fail on?
> > Does that mean at the moment pKVM does out-of-bound access for
> > the header?
> >
> I might have misunderstood the point. I thought you meant the new
> change would cause a panic on most configurations, or were you
> suggesting just removing the WARN_ON?

Just dropping the WARN_ON(), because for most users, that means just
killing the machine (only configurations with debug will give you a
stack trace).

> I can do that, I just updated the current faulty check and left the
> WARN_ON as is.

I'd be all for that.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.