From patchwork Tue Sep 27 17:31:45 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Shmulik Ladkani X-Patchwork-Id: 675748 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3sk7FP2V1Qz9s8x for ; Wed, 28 Sep 2016 03:32:25 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b=hRr/BqNY; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755505AbcI0RcT (ORCPT ); Tue, 27 Sep 2016 13:32:19 -0400 Received: from mail-wm0-f68.google.com ([74.125.82.68]:34069 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751064AbcI0RcQ (ORCPT ); Tue, 27 Sep 2016 13:32:16 -0400 Received: by mail-wm0-f68.google.com with SMTP id l132so2197558wmf.1 for ; Tue, 27 Sep 2016 10:32:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id; bh=k+Vf4pQvbq5mN3I8Mb8KVDJdvuKQrZDFjK2+LasYVZ4=; b=hRr/BqNYidFHSa+kcRN0t/quR2Lm+2Tcan4oxyGKwsnrhZU3RK/MkognplW2r2bY5j uskX6J0qLSoZOOiS17oucQX7X2HStqeWX+BYkI0fESypUm45IbMK99S6+1HCdSfc/FPu BVY5uvxy4gwom+334v8bKuAmgeXaXkqKbczjLese0jUJe88F74OHJkycTj/HJqqaOb1I oTvnq4KkzaoD23cdnmSaRHdYfVwh6GxtGW+Rbqegh51gx5IUfK+9CSk6Kjk9aJAnpQJK 5ANC+s8I3y3KYFDuMpZNxfmQk88oo5H4Zv4R9esHkxRbY+CKj8XZdlebgB9NBTI/hXSs 1I6w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=k+Vf4pQvbq5mN3I8Mb8KVDJdvuKQrZDFjK2+LasYVZ4=; b=e1UxuwX5IkOXxzI0Oz4cu+g0AXk3ZLyTuDRkXFxxM29f57m01vIbjkuE/+P9mHPOQ5 09GUzAJkvofqTA7DIaI0t3ZAM2x6/0JsS93/wkE+VhpmXxfn+3zrWrBKPCO1flp7PJII ojCdAos4BoYyv++mQyEnYD4UY4/WG1bHfJ84BBCuZq5slEKO4lzTWpt7kd6xMFZIJx8J nB7hSUsGEey1Fms13fm06us50dJ7aWN6Mu1cdXUHb1ueu37+wQ+pob9HQCw7jTwsf6is arlJH8SwABqTrbXroxcvl/szoDBbye3HR/SmZBzgdwts0KErX69kJ8lg0Sn4hxW6d4VQ PGxQ== X-Gm-Message-State: AA6/9RlubcAqSB2OQRNLrYUPe2h1OeE/sArMrbj6SAGwUawUHPIKCUsSe8n1JYM19MuuHg== X-Received: by 10.28.66.1 with SMTP id p1mr4023473wma.53.1474997534968; Tue, 27 Sep 2016 10:32:14 -0700 (PDT) Received: from halley.home ([5.102.253.48]) by smtp.gmail.com with ESMTPSA id e187sm4195454wma.21.2016.09.27.10.32.13 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 27 Sep 2016 10:32:14 -0700 (PDT) From: Shmulik Ladkani To: "David S . Miller" , Pravin Shelar Cc: Jiri Pirko , Daniel Borkmann , netdev@vger.kernel.org, Shmulik Ladkani Subject: [PATCH net] net: skbuff: Fix incorrect skb->mac_len adjustment in skb_vlan_push() Date: Tue, 27 Sep 2016 20:31:45 +0300 Message-Id: <1474997505-5059-1-git-send-email-shmulik.ladkani@gmail.com> X-Mailer: git-send-email 2.7.4 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org In case 'skb_vlan_push' is called on an skb with a hw-accel vlan tag already present, the existing hw-accel tag is inserted into payload, and the new given tag is placed as new hw-accel tag. After the insertion: - 'mac_header' is adjusted to point to the new start of the vlan_ethhdr - 'data' is adjusted to point to the vlan_hdr portion (since packet's payload is the inner 802.1q) However, 'mac_len' is incorrectly incremented with additional VLAN_HLEN bytes, resulting in a total value of 18 bytes. Meaning, when issuing 'skb_push(skb, skb->mac_len)' the data points to random content, 4 bytes PRIOR the ethhdr location. This is problematic, as many constructs in the stack are issuing 'skb_push(skb, skb->mac_len)' prior xmit to a device (e.g tcf_mirred, tcf_bpf, nf_dup_netdev_egress), resulting in bogus frames being xmitted (having random 4 bytes at start of frame). For example: # ip l add dev d0 type dummy # tc filter add dev eth0 parent ffff: pref 1 basic \ action vlan push protocol 802.1ad id 5 pipe \ action mirred egress redirect dev d0 Any 802.1q (hw-accel) tagged frames arriving on eth0 are xmitted as bogus frames on d0; whereas the expected behavior is having QinQ frames. Fix, removing the unnecessary VLAN_HLEN adjustment of mac_len. Fixes: 93515d53b1 ("net: move vlan pop/push functions into common code") Signed-off-by: Shmulik Ladkani Cc: Pravin Shelar Cc: Jiri Pirko --- - David, if patch ok, suggest this goes to -stable - Pravin, original push_vlan() code in openvswitch/actions.c prior Jiri has moved it into skbuff.c had the following comment: /* Update mac_len for subsequent MPLS actions */ skb->mac_len += VLAN_HLEN; Can you please acknowlegde OvS code is also ok with suggested change? net/core/skbuff.c | 1 - 1 file changed, 1 deletion(-) diff --git a/net/core/skbuff.c b/net/core/skbuff.c index d36c754895..0cf961868b 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -4607,7 +4607,6 @@ int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci) } skb->protocol = skb->vlan_proto; - skb->mac_len += VLAN_HLEN; skb_postpush_rcsum(skb, skb->data + (2 * ETH_ALEN), VLAN_HLEN); __skb_pull(skb, offset);