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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190403091830.5fa867aa@carbon>
Date:   Wed, 3 Apr 2019 09:18:30 +0200
From:   Jesper Dangaard Brouer <brouer@...hat.com>
To:     Matteo Croce <mcroce@...hat.com>
Cc:     brouer@...hat.com, netdev@...r.kernel.org,
        Sunil Goutham <sgoutham@...ium.com>,
        Robert Richter <rric@...nel.org>,
        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 Wed,  3 Apr 2019 01:11:36 +0200
Matteo Croce <mcroce@...hat.com> wrote:

> 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.

I find this 1500 bytes limit strange.  While XDP does not support
frames that is split over multiple pages, it does support larger frames,
as long as it can fit within one (e.g 4K) page, minus XDP_PACKET_HEADROOM
(256B) and have tail-room for skb_shared_info (320 bytes), which is
4096-256-320 = 3520 bytes.

A quick look at this driver it seems you are limited to 2176 bytes
(RCV_FRAG_LEN=1536+64+320 + 256) based on how the dma mapping is done.
I think the drivers intent is to limit to 1536 bytes, but the DMA
mapping area include more.  It seems rather suboptimal that the
skb_shared_info is included as part of the DMA mapping.


> Fixes: 05c773f52b96e ("net: thunderx: Add basic XDP support")
> Signed-off-by: Matteo Croce <mcroce@...hat.com>
> ---
>  drivers/net/ethernet/cavium/thunder/nicvf_main.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
> index aa2be4807191..fe1d7513f01d 100644
> --- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
> +++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
> @@ -1578,6 +1578,13 @@ static int nicvf_change_mtu(struct net_device *netdev, int new_mtu)
>  	struct nicvf *nic = netdev_priv(netdev);
>  	int orig_mtu = netdev->mtu;
>  
> +	/* For now just support only the usual MTU sized frames */
> +	if (nic->xdp_prog && new_mtu > 1500) {
> +		netdev_warn(netdev, "Jumbo frames not yet supported with XDP, current MTU %d.\n",
> +			    netdev->mtu);
> +		return -EOPNOTSUPP;
> +	}
> +
>  	netdev->mtu = new_mtu;
>  
>  	if (!netif_running(netdev))



-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer