netdev - Re: [PATCH net] net: thunderx: don't allow jumbo frames with XDP

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: <CAGnkfhxB2rGyu4HjnhWPuegx_YLNhPt4d5cRrcaJ34OhWQfq8w@mail.gmail.com>
Date:   Fri, 5 Apr 2019 02:51:18 +0200
From:   Matteo Croce <mcroce@...hat.com>
To:     David Miller <davem@...emloft.net>,
        Jesper Dangaard Brouer <brouer@...hat.com>
Cc:     netdev <netdev@...r.kernel.org>,
        Sunil Goutham <sgoutham@...ium.com>,
        Robert Richter <rric@...nel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Ilias Apalodimas <ilias.apalodimas@...aro.org>
Subject: Re: [PATCH net] net: thunderx: don't allow jumbo frames with XDP

On Fri, Apr 5, 2019 at 2:20 AM David Miller <davem@...emloft.net> wrote:
>
> From: Matteo Croce <mcroce@...hat.com>
> Date: Wed,  3 Apr 2019 01:11:36 +0200
>
> > The thunderx driver forbids to load an eBPF program if the MTU is
> > higher than 1500 bytes, but this can be circumvented by first
> > loading the eBPF, and then raising the MTU.
> >
> > XDP assumes that SKBs are linear and fit in a single page, this can
> > lead to undefined behaviours.
> > Fix this by limiting the MTU to 1500 bytes if an eBPF program is
> > loaded.
> >
> > Fixes: 05c773f52b96e ("net: thunderx: Add basic XDP support")
> > Signed-off-by: Matteo Croce <mcroce@...hat.com>
>
> Please respond to Jesper's feedback about your choice of a limit of
> 1500.
>
> Otherwise I will toss your patch.

Hi David ad Jesper,

I didn't deliberately choose a limit of 1500, the limit is always set
in nicvf_xdp_setup():

    /* For now just support only the usual MTU sized frames */
    if (prog && (dev->mtu > 1500)) {
        netdev_warn(dev, "Jumbo frames not yet supported with XDP...

I just enforced the same limit in another code path which didn't do
the check.
If you think that 1500 is a bad value, and I'm sure you're right because
there isn't room even for VLAN tagging, I will send a series like:
- 1/2 sets the limit to a resonable value
- 2/2 enforce the same limit in the two code paths

Regards,
-- 
Matteo Croce
per aspera ad upstream