linux-kernel - Re: [PATCH v3] KVM: arm64: Add memory length checks and remove inline in do_ffa_mem

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <rm2pihr27elmdf4zcgrv5khs7qluhn77tkivkr6xkvqcybtl4m@7hesctcacm4h>
Date: Wed, 11 Sep 2024 00:32:29 +0800
From: Wei-Lin Chang <r09922117@...e.ntu.edu.tw>
To: Snehal Koukuntla <snehalreddy@...gle.com>, 
	Marc Zyngier <maz@...nel.org>, Oliver Upton <oliver.upton@...ux.dev>
Cc: James Morse <james.morse@....com>, 
	Suzuki K Poulose <suzuki.poulose@....com>, Zenghui Yu <yuzenghui@...wei.com>, 
	Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>, 
	Sudeep Holla <sudeep.holla@....com>, Sebastian Ene <sebastianene@...gle.com>, 
	Vincent Donnefort <vdonnefort@...gle.com>, Jean-Philippe Brucker <jean-philippe@...aro.org>, 
	linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.linux.dev, linux-kernel@...r.kernel.org, 
	stable@...r.kernel.org, r09922117@...e.ntu.edu.tw
Subject: Re: [PATCH v3] KVM: arm64: Add memory length checks and remove
 inline in do_ffa_mem_xfer

Hi everyone,

On Mon, Sep 09, 2024 at 06:01:54PM GMT, Snehal Koukuntla wrote:
> 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.
> 
> Also do_ffa_mem_xfer() does not need __always_inline
> 
> Fixes: 634d90cf0ac65 ("KVM: arm64: Handle FFA_MEM_LEND calls from the host")
> Cc: stable@...r.kernel.org
> Reviewed-by: Sebastian Ene <sebastianene@...gle.com>
> Signed-off-by: Snehal Koukuntla <snehalreddy@...gle.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 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,

I am seeing a compilation error because of this.

----- LOG START -----
$ clang --version 
clang version 18.1.8
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
$ make LLVM=1 ARCH=arm64 defconfig
$ make LLVM=1 ARCH=arm64 Image
...
arch/arm64/kvm/hyp/nvhe/ffa.c:443:2: error: call to '__compiletime_assert_776' declared with 'error' attribute: BUILD_BUG_ON failed: func_id != FFA_FN64_MEM_SHARE && func_id != FFA_FN64_MEM_LEND
  443 |         BUILD_BUG_ON(func_id != FFA_FN64_MEM_SHARE &&
      |         ^
./include/linux/build_bug.h:50:2: note: expanded from macro 'BUILD_BUG_ON'
   50 |         BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
      |         ^
./include/linux/build_bug.h:39:37: note: expanded from macro 'BUILD_BUG_ON_MSG'
   39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
      |                                     ^
././include/linux/compiler_types.h:510:2: note: expanded from macro 'compiletime_assert'
  510 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
      |         ^
././include/linux/compiler_types.h:498:2: note: expanded from macro '_compiletime_assert'
  498 |         __compiletime_assert(condition, msg, prefix, suffix)
      |         ^
././include/linux/compiler_types.h:491:4: note: expanded from macro '__compiletime_assert'
  491 |                         prefix ## suffix();                             \
      |                         ^
<scratch space>:66:1: note: expanded from here
   66 | __compiletime_assert_776
      | ^
1 error generated.
make[6]: *** [arch/arm64/kvm/hyp/nvhe/Makefile:46: arch/arm64/kvm/hyp/nvhe/ffa.nvhe.o] Error 1
make[5]: *** [scripts/Makefile.build:485: arch/arm64/kvm/hyp/nvhe] Error 2
make[4]: *** [scripts/Makefile.build:485: arch/arm64/kvm/hyp] Error 2
make[3]: *** [scripts/Makefile.build:485: arch/arm64/kvm] Error 2
make[3]: *** Waiting for unfinished jobs....
----- LOG END -----

Is this expected? Because when I add back the __always_inline, or change the
BUILD_BUG_ON to BUG_ON, it compiles successfully.

Thanks,
Wei-Lin Chang

>  					    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.598.g6f2099f65c-goog
>