Re: [PATCH v2] KVM: arm64: Add memory length checks and remove inline in do_ffa_mem_xfer

[Sebastian Ene]

On Mon, Sep 09, 2024 at 01:08:32PM +0000, Snehal Koukuntla wrote:

Hi,

A couple of nits which weren't addressed in the first version.

> Check size during allocation to fix discrepancy in memory reclaim path.
> Currently only happens during memory reclaim, inconsistent with mem_xfer
> Also  do_ffa_mem_xfer() does not need __always_inline
> 

Please enhance the description in the commit message as Mark suggested
previously. Something like this should work:

"""
When we share memory through FF-A and the description of the buffers
exceeds the size of the mapped buffer, the fragmentation API is used.
The fragmentation API allows specifying chunks of descriptors in
subsequent FF-A fragment calls and no upper limit has been established
for this. The entire memory region transferred is
identified by a handle which can be used to reclaim the transferred
memory. To be able to reclaim the memory, the description of the buffers
has to fit in the ffa_desc_buf.
Add a bounds check on the FF-A sharing path to prevent the memory reclaim
from failing.

Fixes: 43609000177625
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Snehal Koukuntla <snehalreddy@xxxxxxxxxx>
"""

Otherwise, Reviewed-by: Sebastian Ene <sebastianene@xxxxxxxxxx>

> Signed-off-by: Snehal Koukuntla <snehalreddy@xxxxxxxxxx>
> ---
>  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 e715c157c2c4..637425f63fd1 100644
> --- a/arch/arm64/kvm/hyp/nvhe/ffa.c
> +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
> @@ -426,7 +426,7 @@ static void do_ffa_mem_frag_tx(struct arm_smccc_res *res,
>  	return;
>  }
>  
> -static __always_inline void do_ffa_mem_xfer(const u64 func_id,
> +static void do_ffa_mem_xfer(const u64 func_id,
>  					    struct arm_smccc_res *res,
>  					    struct kvm_cpu_context *ctxt)
>  {
> @@ -461,6 +461,11 @@ static __always_inline void do_ffa_mem_xfer(const u64 func_id,
>  		goto out_unlock;
>  	}
>  
> +	if (len > ffa_desc_buf.len) {
> +		ret = FFA_RET_NO_MEMORY;
> +		goto out_unlock;
> +	}
> +
>  	buf = hyp_buffers.tx;
>  	memcpy(buf, host_buffers.tx, fraglen);
>  
> -- 
> 2.46.0.469.g59c65b2a67-goog
> 

Thanks,
Seb