netdev - Re: [PATCH bpf-next v2 2/8] bpf: XDP metadata RX kfuncs

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]

Date:   Wed, 23 Nov 2022 10:43:08 -0800
From:   Stanislav Fomichev <sdf@...gle.com>
To:     Martin KaFai Lau <martin.lau@...ux.dev>
Cc:     ast@...nel.org, daniel@...earbox.net, andrii@...nel.org,
        song@...nel.org, yhs@...com, john.fastabend@...il.com,
        kpsingh@...nel.org, haoluo@...gle.com, jolsa@...nel.org,
        David Ahern <dsahern@...il.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Willem de Bruijn <willemb@...gle.com>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Anatoly Burakov <anatoly.burakov@...el.com>,
        Alexander Lobakin <alexandr.lobakin@...el.com>,
        Magnus Karlsson <magnus.karlsson@...il.com>,
        Maryam Tahhan <mtahhan@...hat.com>, xdp-hints@...-project.net,
        netdev@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [PATCH bpf-next v2 2/8] bpf: XDP metadata RX kfuncs

On Tue, Nov 22, 2022 at 10:34 PM Martin KaFai Lau <martin.lau@...ux.dev> wrote:
>
> On 11/21/22 10:25 AM, Stanislav Fomichev wrote:
> > --- a/kernel/bpf/core.c
> > +++ b/kernel/bpf/core.c
> > @@ -2576,6 +2576,7 @@ static void bpf_prog_free_deferred(struct work_struct *work)
> >       } else {
> >               bpf_jit_free(aux->prog);
> >       }
> > +     dev_put(aux->xdp_netdev);
>
> I think dev_put needs to be done during unregister_netdevice event also.
> Otherwise, a loaded bpf prog may hold the dev for a long time.  May be there is
> ideas in offload.c.

Let me try to play with a veth pair to make sure the proper cleanup triggers.
I see your point that we now seemingly have to detach/unload the
program to trigger netdev cleanup..

> [ ... ]
>
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index 35972afb6850..ece7f9234b2d 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -2491,7 +2491,8 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr)
> >                                BPF_F_TEST_STATE_FREQ |
> >                                BPF_F_SLEEPABLE |
> >                                BPF_F_TEST_RND_HI32 |
> > -                              BPF_F_XDP_HAS_FRAGS))
> > +                              BPF_F_XDP_HAS_FRAGS |
> > +                              BPF_F_XDP_HAS_METADATA))
> >               return -EINVAL;
> >
> >       if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) &&
> > @@ -2579,6 +2580,20 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr)
> >       prog->aux->sleepable = attr->prog_flags & BPF_F_SLEEPABLE;
> >       prog->aux->xdp_has_frags = attr->prog_flags & BPF_F_XDP_HAS_FRAGS;
> >
> > +     if (attr->prog_flags & BPF_F_XDP_HAS_METADATA) {
> > +             /* Reuse prog_ifindex to bind to the device
> > +              * for XDP metadata kfuncs.
> > +              */
> > +             prog->aux->offload_requested = false;
> > +
> > +             prog->aux->xdp_netdev = dev_get_by_index(current->nsproxy->net_ns,
> > +                                                      attr->prog_ifindex);
> > +             if (!prog->aux->xdp_netdev) {
> > +                     err = -EINVAL;
> > +                     goto free_prog;
> > +             }
> > +     }
>