From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Duyck Subject: Re: [Intel-wired-lan] [PATCH net/jkirsher] bpf, xdp, i40e: fix i40e_build_skb skb reserve and truesize Date: Wed, 13 Jun 2018 10:10:21 -0700 Message-ID: References: <20180613090436.4266-1-daniel@iogearbox.net> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Cc: Daniel Borkmann , Jeff Kirsher , Netdev , =?UTF-8?B?QmrDtnJuIFTDtnBlbA==?= , intel-wired-lan , keith.busch@linux.intel.com, makita.toshiaki@lab.ntt.co.jp To: John Fastabend Return-path: Received: from mail-ot0-f193.google.com ([74.125.82.193]:39994 "EHLO mail-ot0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934817AbeFMRKW (ORCPT ); Wed, 13 Jun 2018 13:10:22 -0400 Received: by mail-ot0-f193.google.com with SMTP id w9-v6so3810417otj.7 for ; Wed, 13 Jun 2018 10:10:22 -0700 (PDT) In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On Wed, Jun 13, 2018 at 9:26 AM, John Fastabend wrote: > On 06/13/2018 02:04 AM, Daniel Borkmann wrote: >> Using skb_reserve(skb, I40E_SKB_PAD + (xdp->data - xdp->data_hard_start)= ) >> is clearly wrong since I40E_SKB_PAD already points to the offset where >> the original xdp->data was sitting since xdp->data_hard_start is defined >> as xdp->data - i40e_rx_offset(rx_ring) where latter offsets to I40E_SKB_= PAD >> when build skb is used. >> >> However, also before cc5b114dcf98 ("bpf, i40e: add meta data support") >> this seems broken since bpf_xdp_adjust_head() helper could have been use= d >> to alter headroom and enlarge / shrink the frame and with that the assum= ption >> that the xdp->data remains unchanged does not hold and would push a bogu= s >> packet to upper stack. >> >> ixgbe got this right in 924708081629 ("ixgbe: add XDP support for pass a= nd >> drop actions"). In any case, fix it by removing the I40E_SKB_PAD from bo= th >> skb_reserve() and truesize calculation. >> >> Fixes: cc5b114dcf98 ("bpf, i40e: add meta data support") >> Fixes: 0c8493d90b6b ("i40e: add XDP support for pass and drop actions") >> Reported-by: Keith Busch >> Reported-by: Toshiaki Makita >> Signed-off-by: Daniel Borkmann >> Cc: Bj=C3=B6rn T=C3=B6pel >> Cc: John Fastabend >> --- >> drivers/net/ethernet/intel/i40e/i40e_txrx.c | 7 +++---- >> 1 file changed, 3 insertions(+), 4 deletions(-) >> > > Thanks! I missed this during review. > > Acked-by: John Fastabend Looks good to me. Thanks for taking care of this. Acked-by: Alexander Duyck From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Duyck Date: Wed, 13 Jun 2018 10:10:21 -0700 Subject: [Intel-wired-lan] [PATCH net/jkirsher] bpf, xdp, i40e: fix i40e_build_skb skb reserve and truesize In-Reply-To: References: <20180613090436.4266-1-daniel@iogearbox.net> Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: intel-wired-lan@osuosl.org List-ID: On Wed, Jun 13, 2018 at 9:26 AM, John Fastabend wrote: > On 06/13/2018 02:04 AM, Daniel Borkmann wrote: >> Using skb_reserve(skb, I40E_SKB_PAD + (xdp->data - xdp->data_hard_start)) >> is clearly wrong since I40E_SKB_PAD already points to the offset where >> the original xdp->data was sitting since xdp->data_hard_start is defined >> as xdp->data - i40e_rx_offset(rx_ring) where latter offsets to I40E_SKB_PAD >> when build skb is used. >> >> However, also before cc5b114dcf98 ("bpf, i40e: add meta data support") >> this seems broken since bpf_xdp_adjust_head() helper could have been used >> to alter headroom and enlarge / shrink the frame and with that the assumption >> that the xdp->data remains unchanged does not hold and would push a bogus >> packet to upper stack. >> >> ixgbe got this right in 924708081629 ("ixgbe: add XDP support for pass and >> drop actions"). In any case, fix it by removing the I40E_SKB_PAD from both >> skb_reserve() and truesize calculation. >> >> Fixes: cc5b114dcf98 ("bpf, i40e: add meta data support") >> Fixes: 0c8493d90b6b ("i40e: add XDP support for pass and drop actions") >> Reported-by: Keith Busch >> Reported-by: Toshiaki Makita >> Signed-off-by: Daniel Borkmann >> Cc: Bj?rn T?pel >> Cc: John Fastabend >> --- >> drivers/net/ethernet/intel/i40e/i40e_txrx.c | 7 +++---- >> 1 file changed, 3 insertions(+), 4 deletions(-) >> > > Thanks! I missed this during review. > > Acked-by: John Fastabend Looks good to me. Thanks for taking care of this. Acked-by: Alexander Duyck