netdev - Re: [PATCH bpf-next v5 07/13] bpf: net

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: <CAADnVQKBe89WSjwsMaaGGmHAtGSSvCVQ+f7HstjQBzx8pu2gUQ@mail.gmail.com>
Date: Fri, 14 Mar 2025 13:24:08 -0700
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Amery Hung <ameryhung@...il.com>
Cc: Network Development <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>, 
	Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>, 
	Martin KaFai Lau <martin.lau@...nel.org>, Jakub Kicinski <kuba@...nel.org>, 
	Eric Dumazet <edumazet@...gle.com>, Cong Wang <xiyou.wangcong@...il.com>, 
	Jamal Hadi Salim <jhs@...atatu.com>, Kui-Feng Lee <sinquersw@...il.com>, 
	Toke Høiland-Jørgensen <toke@...hat.com>, 
	Jiri Pirko <jiri@...nulli.us>, Stanislav Fomichev <stfomichev@...il.com>, 
	ekarani.silvestre@....ufcg.edu.br, yangpeihao@...u.edu.cn, 
	Peilin Ye <yepeilin.cs@...il.com>, Kernel Team <kernel-team@...a.com>
Subject: Re: [PATCH bpf-next v5 07/13] bpf: net_sched: Support updating qstats

On Thu, Mar 13, 2025 at 12:03 PM Amery Hung <ameryhung@...il.com> wrote:
>
> From: Amery Hung <amery.hung@...edance.com>
>
> Allow bpf qdisc programs to update Qdisc qstats directly with btf struct
> access.
>
> Signed-off-by: Amery Hung <amery.hung@...edance.com>
> ---
>  net/sched/bpf_qdisc.c | 53 ++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 45 insertions(+), 8 deletions(-)
>
> diff --git a/net/sched/bpf_qdisc.c b/net/sched/bpf_qdisc.c
> index edf01f3f1c2a..6ad3050275a4 100644
> --- a/net/sched/bpf_qdisc.c
> +++ b/net/sched/bpf_qdisc.c
> @@ -36,6 +36,7 @@ bpf_qdisc_get_func_proto(enum bpf_func_id func_id,
>         }
>  }
>
> +BTF_ID_LIST_SINGLE(bpf_qdisc_ids, struct, Qdisc)
>  BTF_ID_LIST_SINGLE(bpf_sk_buff_ids, struct, sk_buff)
>  BTF_ID_LIST_SINGLE(bpf_sk_buff_ptr_ids, struct, bpf_sk_buff_ptr)
>
> @@ -60,20 +61,37 @@ static bool bpf_qdisc_is_valid_access(int off, int size,
>         return bpf_tracing_btf_ctx_access(off, size, type, prog, info);
>  }
>
> -static int bpf_qdisc_btf_struct_access(struct bpf_verifier_log *log,
> -                                       const struct bpf_reg_state *reg,
> -                                       int off, int size)
> +static int bpf_qdisc_qdisc_access(struct bpf_verifier_log *log,
> +                                 const struct bpf_reg_state *reg,
> +                                 int off, int size)

Introducing this func in patch 3 and refactoring in patch 7 ?
pls avoid the churn.
squash it ?

if (off + size > end) check wouldn't need to be duplicated.
Can get the name of struct from btf for bpf_log() purpose.

>  {
> -       const struct btf_type *t, *skbt;
>         size_t end;
>
> -       skbt = btf_type_by_id(reg->btf, bpf_sk_buff_ids[0]);
> -       t = btf_type_by_id(reg->btf, reg->btf_id);
> -       if (t != skbt) {
> -               bpf_log(log, "only read is supported\n");
> +       switch (off) {
> +       case offsetof(struct Qdisc, qstats) ... offsetofend(struct Qdisc, qstats) - 1:
> +               end = offsetofend(struct Qdisc, qstats);
> +               break;
> +       default:
> +               bpf_log(log, "no write support to Qdisc at off %d\n", off);
> +               return -EACCES;
> +       }
> +
> +       if (off + size > end) {
> +               bpf_log(log,
> +                       "write access at off %d with size %d beyond the member of Qdisc ended at %zu\n",
> +                       off, size, end);
>                 return -EACCES;
>         }
>
> +       return 0;
> +}
> +
> +static int bpf_qdisc_sk_buff_access(struct bpf_verifier_log *log,
> +                                   const struct bpf_reg_state *reg,
> +                                   int off, int size)
> +{
> +       size_t end;
> +
>         switch (off) {
>         case offsetof(struct sk_buff, tstamp):
>                 end = offsetofend(struct sk_buff, tstamp);
> @@ -115,6 +133,25 @@ static int bpf_qdisc_btf_struct_access(struct bpf_verifier_log *log,
>         return 0;
>  }
>
> +static int bpf_qdisc_btf_struct_access(struct bpf_verifier_log *log,
> +                                      const struct bpf_reg_state *reg,
> +                                      int off, int size)
> +{
> +       const struct btf_type *t, *skbt, *qdisct;
> +
> +       skbt = btf_type_by_id(reg->btf, bpf_sk_buff_ids[0]);
> +       qdisct = btf_type_by_id(reg->btf, bpf_qdisc_ids[0]);
> +       t = btf_type_by_id(reg->btf, reg->btf_id);
> +
> +       if (t == skbt)
> +               return bpf_qdisc_sk_buff_access(log, reg, off, size);
> +       else if (t == qdisct)
> +               return bpf_qdisc_qdisc_access(log, reg, off, size);
> +
> +       bpf_log(log, "only read is supported\n");
> +       return -EACCES;
> +}
> +
>  BTF_ID_LIST(bpf_qdisc_init_prologue_ids)
>  BTF_ID(func, bpf_qdisc_init_prologue)
>
> --
> 2.47.1
>