Received-SPF: pass (google.com: domain of linux-kernel+bounces-177842-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: Puranjay Mohan <puranjay12@gmail.com>
To: Maxwell Bland <mbland@motorola.com>, "open list:BPF [GENERAL] (Safe
 Dynamic Programs and Tools)" <bpf@vger.kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>, Will Deacon
 <will@kernel.org>, Alexei Starovoitov <ast@kernel.org>, Daniel Borkmann
 <daniel@iogearbox.net>, Andrii Nakryiko <andrii@kernel.org>, Martin KaFai
 Lau <martin.lau@linux.dev>, Eduard Zingerman <eddyz87@gmail.com>, Song Liu
 <song@kernel.org>, Yonghong Song <yonghong.song@linux.dev>, John Fastabend
 <john.fastabend@gmail.com>, KP Singh <kpsingh@kernel.org>, Stanislav
 Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>, Jiri Olsa
 <jolsa@kernel.org>, Zi Shen Lim <zlim.lnx@gmail.com>, Mark Rutland
 <mark.rutland@arm.com>, Suzuki K Poulose <suzuki.poulose@arm.com>, Mark
 Brown <broonie@kernel.org>, linux-arm-kernel@lists.infradead.org, open
 list <linux-kernel@vger.kernel.org>, Josh Poimboeuf <jpoimboe@kernel.org>
Subject: Re: [PATCH bpf-next v4 2/3] arm64/cfi,bpf: Support kCFI + BPF on arm64
In-Reply-To: <ub6a7msv36rhotqez3usccexkn7kdqqnsyklrnqy7znqas7fhe@cry4jnw3baky>
References: <wtb6czzpvtqq23t4g6hf7on257dtxzdb4fa4nuq3dtq32odmli@xoyyrtthafar>
 <ub6a7msv36rhotqez3usccexkn7kdqqnsyklrnqy7znqas7fhe@cry4jnw3baky>
Date: Mon, 13 May 2024 16:39:28 +0000
Message-ID: <mb61pttj1k6nz.fsf@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

Maxwell Bland <mbland@motorola.com> writes:

This patch has a subtle difference from the patch that I sent in v2[1]

Unfortunately, you didn't test this. :(

It will break BPF on an ARM64 kernel compiled with CONFIG_CFI_CLANG=y

See below:

> diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
> index 76b91f36c729..703247457409 100644
> --- a/arch/arm64/net/bpf_jit_comp.c
> +++ b/arch/arm64/net/bpf_jit_comp.c
> @@ -17,6 +17,7 @@
>  #include <asm/asm-extable.h>
>  #include <asm/byteorder.h>
>  #include <asm/cacheflush.h>
> +#include <asm/cfi.h>
>  #include <asm/debug-monitors.h>
>  #include <asm/insn.h>
>  #include <asm/patching.h>
> @@ -162,6 +163,12 @@ static inline void emit_bti(u32 insn, struct jit_ctx *ctx)
>  		emit(insn, ctx);
>  }
>  
> +static inline void emit_kcfi(u32 hash, struct jit_ctx *ctx)
> +{
> +	if (IS_ENABLED(CONFIG_CFI_CLANG))
> +		emit(hash, ctx);
> +}
> +
>  /*
>   * Kernel addresses in the vmalloc space use at most 48 bits, and the
>   * remaining bits are guaranteed to be 0x1. So we can compose the address
> @@ -337,6 +344,7 @@ static int build_prologue(struct jit_ctx *ctx, bool ebpf_from_cbpf,
>  	 *
>  	 */

In my original patch the hunk here looked something like:

--- >8 ---

-	const int idx0 = ctx->idx;
 	int cur_offset;
 
 	/*
@@ -332,6 +338,8 @@ static int build_prologue(struct jit_ctx *ctx, bool ebpf_from_cbpf,
 	 *
 	 */
 
+	emit_kcfi(is_subprog ? cfi_bpf_subprog_hash : cfi_bpf_hash, ctx);
+	const int idx0 = ctx->idx;

--- 8< ---

moving idx0 = ctx->idx; after emit_kcfi() is important because later
this 'idx0' is used like:

   cur_offset = ctx->idx - idx0;
   if (cur_offset != PROLOGUE_OFFSET) {
           pr_err_once("PROLOGUE_OFFSET = %d, expected %d!\n",
                       cur_offset, PROLOGUE_OFFSET);
           return -1;
   }

With the current version, when I boot the kernel I get:

[    0.499207] bpf_jit: PROLOGUE_OFFSET = 13, expected 12!

and now no BPF program can be JITed!

Please fix this in the next version and test it by running:

/tools/testing/selftests/bpf/test_progs

Pay attention to the `rbtree_success` and the `dummy_st_ops` tests, they
are the important ones for this change.

[1] https://lore.kernel.org/all/20240324211518.93892-2-puranjay12@gmail.com/

Thanks,
Puranjay