linux-kernel - Re: [PATCH bpf-next v3 1/2] bpf: Add arm64 JIT support for PROBE

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] [day] [month] [year] [list]

Message-ID: <CAADnVQ+-Wt=OohzVJL9g3r2pLkaomGXDLLdLdGeGYE4D+f4cAA@mail.gmail.com>
Date: Sat, 23 Mar 2024 11:48:34 -0700
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Puranjay Mohan <puranjay12@...il.com>
Cc: Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, 
	John Fastabend <john.fastabend@...il.com>, Andrii Nakryiko <andrii@...nel.org>, 
	Martin KaFai Lau <martin.lau@...ux.dev>, Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>, 
	Yonghong Song <yonghong.song@...ux.dev>, KP Singh <kpsingh@...nel.org>, 
	Stanislav Fomichev <sdf@...gle.com>, Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>, 
	bpf <bpf@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>, 
	Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>, 
	Zi Shen Lim <zlim.lnx@...il.com>, Xu Kuohai <xukuohai@...wei.com>
Subject: Re: [PATCH bpf-next v3 1/2] bpf: Add arm64 JIT support for
 PROBE_MEM32 pseudo instructions.

On Sat, Mar 23, 2024 at 3:31 AM Puranjay Mohan <puranjay12@...il.com> wrote:
>
> +#define PROBE_MEM32_BASE (MAX_BPF_JIT_REG + 5)
>
>  #define check_imm(bits, imm) do {                              \
>         if ((((imm) > 0) && ((imm) >> (bits))) ||               \
> @@ -67,6 +68,8 @@ static const int bpf2a64[] = {
>         /* temporary register for blinding constants */
>         [BPF_REG_AX] = A64_R(9),
>         [FP_BOTTOM] = A64_R(27),
> +       /* callee saved register for kern_vm_start address */
> +       [PROBE_MEM32_BASE] = A64_R(28),
>  };
>
>  struct jit_ctx {
> @@ -295,7 +298,7 @@ static bool is_lsi_offset(int offset, int scale)
>  #define PROLOGUE_OFFSET (BTI_INSNS + 2 + PAC_INSNS + 8)
>
>  static int build_prologue(struct jit_ctx *ctx, bool ebpf_from_cbpf,
> -                         bool is_exception_cb)
> +                         bool is_exception_cb, u64 arena_vm_start)
>  {
>         const struct bpf_prog *prog = ctx->prog;
>         const bool is_main_prog = !bpf_is_subprog(prog);
> @@ -306,6 +309,7 @@ static int build_prologue(struct jit_ctx *ctx, bool ebpf_from_cbpf,
>         const u8 fp = bpf2a64[BPF_REG_FP];
>         const u8 tcc = bpf2a64[TCALL_CNT];
>         const u8 fpb = bpf2a64[FP_BOTTOM];
> +       const u8 pb = bpf2a64[PROBE_MEM32_BASE];

In addition to riscv comments please use more verbose name here.
'pb' is too cryptic.
'mem32_base' ?

I would also drop PROBE prefix and use:
#define MEM32_BASE (MAX_BPF_JIT_REG + 5)

>From the verifier pov the ld/st mode is BPF_PROBE_MEM32,
since it's asking JIT to emit code to probe read/write such arena address,
but from JIT pov the base is a real base that it got from
bpf_arena_get_kern_vm_start().
#define KERN_VM_START (MAX_BPF_JIT_REG + 5)
would be an alternative name that also fits.
or
#define ARENA_VM_START ...