linux-kernel - Re: [PATCH v4] netdev attribute to control xdpgeneric skb linearization

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: <CAMOZA0KnC0n+U+n3vjx4bb6o_1_MtzLxUfoGj22NdNVQO33uAA@mail.gmail.com>
Date:   Fri, 28 Feb 2020 05:19:54 -0800
From:   Luigi Rizzo <lrizzo@...gle.com>
To:     Jesper Dangaard Brouer <jbrouer@...hat.com>
Cc:     netdev@...r.kernel.org,
        Toke Høiland-Jørgensen <toke@...hat.com>,
        David Miller <davem@...emloft.net>,
        Jesper Dangaard Brouer <hawk@...nel.org>, sameehj@...zon.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4] netdev attribute to control xdpgeneric skb linearization

On Fri, Feb 28, 2020 at 4:30 AM Jesper Dangaard Brouer
<jbrouer@...hat.com> wrote:
>
> On Fri, 28 Feb 2020 02:54:35 -0800
> Luigi Rizzo <lrizzo@...gle.com> wrote:
>
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index dbbfff123196..c539489d3166 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -4520,9 +4520,12 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
> >       /* XDP packets must be linear and must have sufficient headroom
> >        * of XDP_PACKET_HEADROOM bytes. This is the guarantee that also
> >        * native XDP provides, thus we need to do it here as well.
> > +      * For non shared skbs, xdpgeneric_linearize controls linearization.
> >        */
> > -     if (skb_cloned(skb) || skb_is_nonlinear(skb) ||
> > -         skb_headroom(skb) < XDP_PACKET_HEADROOM) {
> > +     if (skb_cloned(skb) ||
> > +         (skb->dev->xdpgeneric_linearize &&
> > +          (skb_is_nonlinear(skb) ||
> > +           skb_headroom(skb) < XDP_PACKET_HEADROOM))) {
> >               int hroom = XDP_PACKET_HEADROOM - skb_headroom(skb);
> >               int troom = skb->tail + skb->data_len - skb->end;
> >
>
> Have you checked that calling bpf_xdp_adjust_tail() is not breaking anything?

It won't leak memory or cause crashes if that is what you mean.
Of course if there are more segments the effect won't be the desired one,
as it will chop off the tail of the first segment.

But this is an opt-in feature and requires the same permissions needed to load
an xdp program, so I expect it to be used consciously.

It would be nice if we had a flag in the xdp_buff to communicate that
the packet is
incomplete, but there isn't a way that I can see.

cheers
luigi