| 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
| ||
|
Message-ID: <11481869-52a9-037a-c7a4-ebbc7d426229@huawei.com>
Date: Wed, 10 May 2023 15:06:59 +0800
From: Xu Kuohai <xukuohai@...wei.com>
To: Florent Revest <revest@...omium.org>, <bpf@...r.kernel.org>,
<linux-kernel@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>
CC: <ast@...nel.org>, <daniel@...earbox.net>, <andrii@...nel.org>,
<kpsingh@...nel.org>, <mark.rutland@....com>,
<xukuohai@...weicloud.com>, <zlim.lnx@...il.com>
Subject: Re: [PATCH bpf-next] arm64,bpf: Support struct arguments in the BPF
trampoline
On 5/9/2023 12:46 AM, Florent Revest wrote:
> This extends the BPF trampoline JIT to support attachment to functions
> that take small structures (up to 128bit) as argument. This is trivially
> achieved by saving/restoring a number of "argument registers" rather
> than a number of arguments.
>
> The AAPCS64 section 6.8.2 describes the parameter passing ABI.
> "Composite types" (like C structs) below 16 bytes (as enforced by the
> BPF verifier) are provided as part of the 8 argument registers as
> explained in the section C.12.
>
> Signed-off-by: Florent Revest <revest@...omium.org>
> ---
> arch/arm64/net/bpf_jit_comp.c | 51 ++++++++++----------
> tools/testing/selftests/bpf/DENYLIST.aarch64 | 1 -
> 2 files changed, 26 insertions(+), 26 deletions(-)
>
> diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
> index b26da8efa616..22e3c456554f 100644
> --- a/arch/arm64/net/bpf_jit_comp.c
> +++ b/arch/arm64/net/bpf_jit_comp.c
> @@ -1731,21 +1731,21 @@ static void invoke_bpf_mod_ret(struct jit_ctx *ctx, struct bpf_tramp_links *tl,
> }
> }
>
> -static void save_args(struct jit_ctx *ctx, int args_off, int nargs)
> +static void save_args(struct jit_ctx *ctx, int args_off, int nregs)
> {
> int i;
>
> - for (i = 0; i < nargs; i++) {
> + for (i = 0; i < nregs; i++) {
> emit(A64_STR64I(i, A64_SP, args_off), ctx);
> args_off += 8;
> }
> }
>
> -static void restore_args(struct jit_ctx *ctx, int args_off, int nargs)
> +static void restore_args(struct jit_ctx *ctx, int args_off, int nregs)
> {
> int i;
>
> - for (i = 0; i < nargs; i++) {
> + for (i = 0; i < nregs; i++) {
> emit(A64_LDR64I(i, A64_SP, args_off), ctx);
> args_off += 8;
> }
> @@ -1764,7 +1764,7 @@ static void restore_args(struct jit_ctx *ctx, int args_off, int nargs)
> */
> static int prepare_trampoline(struct jit_ctx *ctx, struct bpf_tramp_image *im,
> struct bpf_tramp_links *tlinks, void *orig_call,
> - int nargs, u32 flags)
> + int nregs, u32 flags)
> {
> int i;
> int stack_size;
> @@ -1772,7 +1772,7 @@ static int prepare_trampoline(struct jit_ctx *ctx, struct bpf_tramp_image *im,
> int regs_off;
> int retval_off;
> int args_off;
> - int nargs_off;
> + int nregs_off;
> int ip_off;
> int run_ctx_off;
> struct bpf_tramp_links *fentry = &tlinks[BPF_TRAMP_FENTRY];
> @@ -1799,7 +1799,7 @@ static int prepare_trampoline(struct jit_ctx *ctx, struct bpf_tramp_image *im,
> * [ ... ]
> * SP + args_off [ arg1 ]
> *
> - * SP + nargs_off [ args count ]
> + * SP + nregs_off [ arg regs count ]
For description consistency, should arg1 ... argN in the previous
lines also be changed to arg reg 1 ... arg reg N?
> *
> * SP + ip_off [ traced function ] BPF_TRAMP_F_IP_ARG flag
> *
> @@ -1816,13 +1816,13 @@ static int prepare_trampoline(struct jit_ctx *ctx, struct bpf_tramp_image *im,
> if (flags & BPF_TRAMP_F_IP_ARG)
> stack_size += 8;
>
> - nargs_off = stack_size;
> + nregs_off = stack_size;
> /* room for args count */
> stack_size += 8;
>
> args_off = stack_size;
> /* room for args */
> - stack_size += nargs * 8;
> + stack_size += nregs * 8;
>
> /* room for return value */
> retval_off = stack_size;
> @@ -1865,12 +1865,12 @@ static int prepare_trampoline(struct jit_ctx *ctx, struct bpf_tramp_image *im,
> emit(A64_STR64I(A64_R(10), A64_SP, ip_off), ctx);
> }
>
> - /* save args count*/
> - emit(A64_MOVZ(1, A64_R(10), nargs, 0), ctx);
> - emit(A64_STR64I(A64_R(10), A64_SP, nargs_off), ctx);
> + /* save arg regs count*/
> + emit(A64_MOVZ(1, A64_R(10), nregs, 0), ctx);
> + emit(A64_STR64I(A64_R(10), A64_SP, nregs_off), ctx);
>
> - /* save args */
> - save_args(ctx, args_off, nargs);
> + /* save arg regs */
> + save_args(ctx, args_off, nregs);
>
> /* save callee saved registers */
> emit(A64_STR64I(A64_R(19), A64_SP, regs_off), ctx);
> @@ -1897,7 +1897,7 @@ static int prepare_trampoline(struct jit_ctx *ctx, struct bpf_tramp_image *im,
> }
>
> if (flags & BPF_TRAMP_F_CALL_ORIG) {
> - restore_args(ctx, args_off, nargs);
> + restore_args(ctx, args_off, nregs);
> /* call original func */
> emit(A64_LDR64I(A64_R(10), A64_SP, retaddr_off), ctx);
> emit(A64_ADR(A64_LR, AARCH64_INSN_SIZE * 2), ctx);
> @@ -1926,7 +1926,7 @@ static int prepare_trampoline(struct jit_ctx *ctx, struct bpf_tramp_image *im,
> }
>
> if (flags & BPF_TRAMP_F_RESTORE_REGS)
> - restore_args(ctx, args_off, nargs);
> + restore_args(ctx, args_off, nregs);
>
> /* restore callee saved register x19 and x20 */
> emit(A64_LDR64I(A64_R(19), A64_SP, regs_off), ctx);
> @@ -1967,24 +1967,25 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
> void *orig_call)
> {
> int i, ret;
> - int nargs = m->nr_args;
> + int nregs = m->nr_args;
> int max_insns = ((long)image_end - (long)image) / AARCH64_INSN_SIZE;
> struct jit_ctx ctx = {
> .image = NULL,
> .idx = 0,
> };
>
> - /* the first 8 arguments are passed by registers */
> - if (nargs > 8)
> - return -ENOTSUPP;
> -
> - /* don't support struct argument */
> + /* extra registers needed for struct argument */
> for (i = 0; i < MAX_BPF_FUNC_ARGS; i++) {
> + /* The arg_size is at most 16 bytes, enforced by the verifier. */
> if (m->arg_flags[i] & BTF_FMODEL_STRUCT_ARG)
> - return -ENOTSUPP;
> + nregs += (m->arg_size[i] + 7) / 8 - 1;
> }
>
> - ret = prepare_trampoline(&ctx, im, tlinks, orig_call, nargs, flags);
> + /* the first 8 registers are used for arguments */
> + if (nregs > 8)
> + return -ENOTSUPP;
> +
> + ret = prepare_trampoline(&ctx, im, tlinks, orig_call, nregs, flags);
> if (ret < 0)
> return ret;
>
> @@ -1995,7 +1996,7 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
> ctx.idx = 0;
>
> jit_fill_hole(image, (unsigned int)(image_end - image));
> - ret = prepare_trampoline(&ctx, im, tlinks, orig_call, nargs, flags);
> + ret = prepare_trampoline(&ctx, im, tlinks, orig_call, nregs, flags);
>
> if (ret > 0 && validate_code(&ctx) < 0)
> ret = -EINVAL;
> diff --git a/tools/testing/selftests/bpf/DENYLIST.aarch64 b/tools/testing/selftests/bpf/DENYLIST.aarch64
> index cd42e2825bd2..08adc805878b 100644
> --- a/tools/testing/selftests/bpf/DENYLIST.aarch64
> +++ b/tools/testing/selftests/bpf/DENYLIST.aarch64
> @@ -10,4 +10,3 @@ kprobe_multi_test/link_api_addrs # link_fd unexpected link_fd: a
> kprobe_multi_test/link_api_syms # link_fd unexpected link_fd: actual -95 < expected 0
> kprobe_multi_test/skel_api # libbpf: failed to load BPF skeleton 'kprobe_multi': -3
> module_attach # prog 'kprobe_multi': failed to auto-attach: -95
> -tracing_struct # tracing_struct__attach unexpected error: -524 (errno 524)
Powered by blists - more mailing lists