linux-kernel - Re: [PATCH bpf-next v6 0/3] bpf, x86: allow function arguments up to 12 for TRACING

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: <f0f3fe74-0d0e-e428-e676-2eb5911bbcfb@meta.com>
Date:   Wed, 21 Jun 2023 09:07:53 -0700
From:   Yonghong Song <yhs@...a.com>
To:     menglong8.dong@...il.com, alexei.starovoitov@...il.com
Cc:     ast@...nel.org, daniel@...earbox.net, andrii@...nel.org,
        martin.lau@...ux.dev, song@...nel.org, yhs@...com,
        john.fastabend@...il.com, kpsingh@...nel.org, sdf@...gle.com,
        haoluo@...gle.com, jolsa@...nel.org, benbjiang@...cent.com,
        bpf@...r.kernel.org, linux-kernel@...r.kernel.org,
        Menglong Dong <imagedong@...cent.com>
Subject: Re: [PATCH bpf-next v6 0/3] bpf, x86: allow function arguments up to
 12 for TRACING



On 6/19/23 4:49 AM, menglong8.dong@...il.com wrote:
> From: Menglong Dong <imagedong@...cent.com>
> 
> For now, the BPF program of type BPF_PROG_TYPE_TRACING can only be used
> on the kernel functions whose arguments count less than 6. This is not

less than or equal to 6, if not considering '> 8 bytes'
struct arguments.

> friendly at all, as too many functions have arguments count more than 6.
> According to the current kernel version, below is a statistics of the
> function arguments count:
> 
> argument count | function count
> 7              | 704
> 8              | 270
> 9              | 84
> 10             | 47
> 11             | 47
> 12             | 27
> 13             | 22
> 14             | 5
> 15             | 0
> 16             | 1
> 
> Therefore, let's enhance it by increasing the function arguments count
> allowed in arch_prepare_bpf_trampoline(), for now, only x86_64.
> 
> In the 1st patch, we save/restore regs with BPF_DW size to make the code
> in save_regs()/restore_regs() simpler.
> 
> In the 2nd patch, we make arch_prepare_bpf_trampoline() support to copy
> function arguments in stack for x86 arch. Therefore, the maximum
> arguments can be up to MAX_BPF_FUNC_ARGS for FENTRY and FEXIT.

for FENTRY, FEXIT and MODIFY_RETURN.

  Meanwhile,
> we clean the potentian garbage value when we copy the arguments on-stack.

potentian -> potential

> 
> And the 3rd patches are for the testcases of the this series.

the 3rd patch is ...

> 
> Changes since v5:
> - adjust the commit log of the 1st patch, avoiding confusing people that
>    bugs exist in current code
> - introduce get_nr_regs() to get the space that used to pass args on
>    stack correct in the 2nd patch
> - add testcases to tracing_struct.c instead of fentry_test.c and
>    fexit_test.c
> 
> Changes since v4:
> - consider the case of the struct in arguments can't be hold by regs
> - add comment for some code
> - add testcases for MODIFY_RETURN
> - rebase to the latest
> 
> Changes since v3:
> - try make the stack pointer 16-byte aligned. Not sure if I'm right :)
> - introduce clean_garbage() to clean the grabage when argument count is 7
> - use different data type in bpf_testmod_fentry_test{7,12}
> - add testcase for grabage values in ctx
> 
> Changes since v2:
> - keep MAX_BPF_FUNC_ARGS still
> - clean garbage value in upper bytes in the 2nd patch
> - move bpf_fentry_test{7,12} to bpf_testmod.c and rename them to
>    bpf_testmod_fentry_test{7,12} meanwhile in the 3rd patch
> 
> Changes since v1:
> - change the maximun function arguments to 14 from 12
> - add testcases (Jiri Olsa)
> - instead EMIT4 with EMIT3_off32 for "lea" to prevent overflow
> 
> Menglong Dong (3):
>    bpf, x86: save/restore regs with BPF_DW size
>    bpf, x86: allow function arguments up to 12 for TRACING
>    selftests/bpf: add testcase for TRACING with 6+ arguments
> 
>   arch/x86/net/bpf_jit_comp.c                   | 249 +++++++++++++++---
>   net/bpf/test_run.c                            |  23 +-
>   .../selftests/bpf/bpf_testmod/bpf_testmod.c   |  49 +++-
>   .../selftests/bpf/prog_tests/fentry_fexit.c   |   4 +-
>   .../selftests/bpf/prog_tests/fentry_test.c    |   2 +
>   .../selftests/bpf/prog_tests/fexit_test.c     |   2 +
>   .../selftests/bpf/prog_tests/modify_return.c  |  20 +-
>   .../selftests/bpf/prog_tests/tracing_struct.c |  19 ++
>   .../testing/selftests/bpf/progs/fentry_test.c |  32 +++
>   .../testing/selftests/bpf/progs/fexit_test.c  |  33 +++
>   .../selftests/bpf/progs/modify_return.c       |  40 +++
>   .../selftests/bpf/progs/tracing_struct.c      |  48 ++++
>   12 files changed, 471 insertions(+), 50 deletions(-)
>