From: Mostafa Saleh smostafa@google.com
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.
Also, drop the WARN_ONs as that will panic the kernel and in the next checks there are no WARNs, so that makes it consistent.
Fixes: 0a9f15fd5674 ("KVM: arm64: pkvm: Add support for fragmented FF-A descriptors") Signed-off-by: Mostafa Saleh smostafa@google.com Reviewed-by: Vincent Donnefort vdonnefort@google.com Signed-off-by: Sebastian Ene sebastianene@google.com --- arch/arm64/kvm/hyp/nvhe/ffa.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c index 1af722771178..41cc4c1bafeb 100644 --- a/arch/arm64/kvm/hyp/nvhe/ffa.c +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c @@ -607,8 +607,8 @@ 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 || - fraglen > KVM_FFA_MBOX_NR_PAGES * PAGE_SIZE)) { + if (offset + CONSTITUENTS_OFFSET(0) > len || + fraglen > KVM_FFA_MBOX_NR_PAGES * PAGE_SIZE) { ret = FFA_RET_ABORTED; ffa_rx_release(res); goto out_unlock; @@ -636,11 +636,16 @@ static void do_ffa_mem_reclaim(struct arm_smccc_1_2_regs *res, ffa_rx_release(res); }
+ reg = (void *)buf + offset; + if (offset + CONSTITUENTS_OFFSET(reg->addr_range_cnt) > len) { + ret = FFA_RET_ABORTED; + goto out_unlock; + } + ffa_mem_reclaim(res, handle_lo, handle_hi, flags); if (res->a0 != FFA_SUCCESS) goto out_unlock;
- reg = (void *)buf + offset; /* If the SPMD was happy, then we should be too. */ WARN_ON(ffa_host_unshare_ranges(reg->constituents, reg->addr_range_cnt));