linux-kernel - Re: [PATCH bpf-next v2 1/5] bpf: make MAX_BPF_FUNC

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: <CADxym3bp-KeFX4Mxo8Xhh9_NUWp5hw_bTQBOhTgJzskH7d_k_A@mail.gmail.com>
Date:   Mon, 5 Jun 2023 10:49:54 +0800
From:   Menglong Dong <menglong8.dong@...il.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Jiri Olsa <olsajiri@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        David Ahern <dsahern@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <martin.lau@...ux.dev>,
        Song Liu <song@...nel.org>, Yonghong Song <yhs@...com>,
        KP Singh <kpsingh@...nel.org>, Jiri Olsa <jolsa@...nel.org>,
        X86 ML <x86@...nel.org>, Biao Jiang <benbjiang@...cent.com>,
        Network Development <netdev@...r.kernel.org>,
        bpf <bpf@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@...r.kernel.org>
Subject: Re: [PATCH bpf-next v2 1/5] bpf: make MAX_BPF_FUNC_ARGS 14

On Sat, Jun 3, 2023 at 2:17 AM Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
>
> On Fri, Jun 2, 2023 at 12:01 AM <menglong8.dong@...il.com> wrote:
> >
> > From: Menglong Dong <imagedong@...cent.com>
> >
> > According to the current kernel version, below is a statistics of the
> > function arguments count:
> >
> > argument count | FUNC_PROTO count
> > 7              | 367
> > 8              | 196
> > 9              | 71
> > 10             | 43
> > 11             | 22
> > 12             | 10
> > 13             | 15
> > 14             | 4
> > 15             | 0
> > 16             | 1
> >
> > It's hard to statisics the function count, so I use FUNC_PROTO in the btf
> > of vmlinux instead. The function with 16 arguments is ZSTD_buildCTable(),
> > which I think can be ignored.
> >
> > Therefore, let's make the maximum of function arguments count 14. It used
> > to be 12, but it seems that there is no harm to make it big enough.
>
> I think we're just fine at 12.
> People need to fix their code. ZSTD_buildCTable should be first in line.
> Passing arguments on the stack is not efficient from performance pov.

But we still need to keep this part:

@@ -2273,7 +2273,8 @@ bool btf_ctx_access(int off, int size, enum
bpf_access_type type,
 static inline bool bpf_tracing_ctx_access(int off, int size,
                                          enum bpf_access_type type)
 {
-       if (off < 0 || off >= sizeof(__u64) * MAX_BPF_FUNC_ARGS)
+       /* "+1" here is for FEXIT return value. */
+       if (off < 0 || off >= sizeof(__u64) * (MAX_BPF_FUNC_ARGS + 1))
                return false;
        if (type != BPF_READ)
                return false;

Isn't it? Otherwise, it will make that the maximum arguments
is 12 for FENTRY, but 11 for FEXIT, as FEXIT needs to store
the return value in ctx.

How about that we change bpf_tracing_ctx_access() into:

static inline bool bpf_tracing_ctx_access(int off, int size,
                      enum bpf_access_type type,
                      int max_args)

And the caller can pass MAX_BPF_FUNC_ARGS to
it on no-FEXIT, and 'MAX_BPF_FUNC_ARGS + 1'
on FEXIT.

What do you think?

Thanks!
Menglong Dong