All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Eyal Birger <eyal.birger@gmail.com>,
	"Jason A. Donenfeld" <Jason@zx2c4.com>,
	Willem de Bruijn <willemb@google.com>,
	Jakub Kicinski <kuba@kernel.org>
Subject: [PATCH 5.9 15/46] net/packet: fix packet receive on L3 devices without visible hard header
Date: Sun,  6 Dec 2020 12:17:23 +0100	[thread overview]
Message-ID: <20201206111557.197220214@linuxfoundation.org> (raw)
In-Reply-To: <20201206111556.455533723@linuxfoundation.org>

From: Eyal Birger <eyal.birger@gmail.com>

[ Upstream commit d549699048b4b5c22dd710455bcdb76966e55aa3 ]

In the patchset merged by commit b9fcf0a0d826
("Merge branch 'support-AF_PACKET-for-layer-3-devices'") L3 devices which
did not have header_ops were given one for the purpose of protocol parsing
on af_packet transmit path.

That change made af_packet receive path regard these devices as having a
visible L3 header and therefore aligned incoming skb->data to point to the
skb's mac_header. Some devices, such as ipip, xfrmi, and others, do not
reset their mac_header prior to ingress and therefore their incoming
packets became malformed.

Ideally these devices would reset their mac headers, or af_packet would be
able to rely on dev->hard_header_len being 0 for such cases, but it seems
this is not the case.

Fix by changing af_packet RX ll visibility criteria to include the
existence of a '.create()' header operation, which is used when creating
a device hard header - via dev_hard_header() - by upper layers, and does
not exist in these L3 devices.

As this predicate may be useful in other situations, add it as a common
dev_has_header() helper in netdevice.h.

Fixes: b9fcf0a0d826 ("Merge branch 'support-AF_PACKET-for-layer-3-devices'")
Signed-off-by: Eyal Birger <eyal.birger@gmail.com>
Acked-by: Jason A. Donenfeld <Jason@zx2c4.com>
Acked-by: Willem de Bruijn <willemb@google.com>
Link: https://lore.kernel.org/r/20201121062817.3178900-1-eyal.birger@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 include/linux/netdevice.h |    5 +++++
 net/packet/af_packet.c    |   38 +++++++++++++++++++++-----------------
 2 files changed, 26 insertions(+), 17 deletions(-)

--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3103,6 +3103,11 @@ static inline bool dev_validate_header(c
 	return false;
 }
 
+static inline bool dev_has_header(const struct net_device *dev)
+{
+	return dev->header_ops && dev->header_ops->create;
+}
+
 typedef int gifconf_func_t(struct net_device * dev, char __user * bufptr,
 			   int len, int size);
 int register_gifconf(unsigned int family, gifconf_func_t *gifconf);
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -93,38 +93,42 @@
 
 /*
    Assumptions:
-   - if device has no dev->hard_header routine, it adds and removes ll header
-     inside itself. In this case ll header is invisible outside of device,
-     but higher levels still should reserve dev->hard_header_len.
-     Some devices are enough clever to reallocate skb, when header
-     will not fit to reserved space (tunnel), another ones are silly
-     (PPP).
+   - If the device has no dev->header_ops->create, there is no LL header
+     visible above the device. In this case, its hard_header_len should be 0.
+     The device may prepend its own header internally. In this case, its
+     needed_headroom should be set to the space needed for it to add its
+     internal header.
+     For example, a WiFi driver pretending to be an Ethernet driver should
+     set its hard_header_len to be the Ethernet header length, and set its
+     needed_headroom to be (the real WiFi header length - the fake Ethernet
+     header length).
    - packet socket receives packets with pulled ll header,
      so that SOCK_RAW should push it back.
 
 On receive:
 -----------
 
-Incoming, dev->hard_header!=NULL
+Incoming, dev_has_header(dev) == true
    mac_header -> ll header
    data       -> data
 
-Outgoing, dev->hard_header!=NULL
+Outgoing, dev_has_header(dev) == true
    mac_header -> ll header
    data       -> ll header
 
-Incoming, dev->hard_header==NULL
-   mac_header -> UNKNOWN position. It is very likely, that it points to ll
-		 header.  PPP makes it, that is wrong, because introduce
-		 assymetry between rx and tx paths.
+Incoming, dev_has_header(dev) == false
+   mac_header -> data
+     However drivers often make it point to the ll header.
+     This is incorrect because the ll header should be invisible to us.
    data       -> data
 
-Outgoing, dev->hard_header==NULL
-   mac_header -> data. ll header is still not built!
+Outgoing, dev_has_header(dev) == false
+   mac_header -> data. ll header is invisible to us.
    data       -> data
 
 Resume
-  If dev->hard_header==NULL we are unlikely to restore sensible ll header.
+  If dev_has_header(dev) == false we are unable to restore the ll header,
+    because it is invisible to us.
 
 
 On transmit:
@@ -2066,7 +2070,7 @@ static int packet_rcv(struct sk_buff *sk
 
 	skb->dev = dev;
 
-	if (dev->header_ops) {
+	if (dev_has_header(dev)) {
 		/* The device has an explicit notion of ll header,
 		 * exported to higher levels.
 		 *
@@ -2195,7 +2199,7 @@ static int tpacket_rcv(struct sk_buff *s
 	if (!net_eq(dev_net(dev), sock_net(sk)))
 		goto drop;
 
-	if (dev->header_ops) {
+	if (dev_has_header(dev)) {
 		if (sk->sk_type != SOCK_DGRAM)
 			skb_push(skb, skb->data - skb_mac_header(skb));
 		else if (skb->pkt_type == PACKET_OUTGOING) {



  parent reply	other threads:[~2020-12-06 11:46 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-06 11:17 [PATCH 5.9 00/46] 5.9.13-rc1 review Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 01/46] devlink: Hold rtnl lock while reading netdev attributes Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 02/46] devlink: Make sure devlink instance and port are in same net namespace Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 03/46] ipv6: addrlabel: fix possible memory leak in ip6addrlbl_net_init Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 04/46] net/af_iucv: set correct sk_protocol for child sockets Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 05/46] net: openvswitch: fix TTL decrement action netlink message format Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 06/46] net/tls: missing received data after fast remote close Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 07/46] net/tls: Protect from calling tls_dev_del for TLS RX twice Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 08/46] rose: Fix Null pointer dereference in rose_send_frame() Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 09/46] sock: set sk_err to ee_errno on dequeue from errq Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 10/46] tcp: Set INET_ECN_xmit configuration in tcp_reinit_congestion_control Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 11/46] tun: honor IOCB_NOWAIT flag Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 12/46] usbnet: ipheth: fix connectivity with iOS 14 Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 13/46] vsock/virtio: discard packets only when socket is really closed Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 14/46] mptcp: fix NULL ptr dereference on bad MPJ Greg Kroah-Hartman
2020-12-06 11:17 ` Greg Kroah-Hartman [this message]
2020-12-06 11:17 ` [PATCH 5.9 16/46] netfilter: bridge: reset skb->pkt_type after NF_INET_POST_ROUTING traversal Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 17/46] ipv4: Fix tos mask in inet_rtm_getroute() Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 18/46] dt-bindings: net: correct interrupt flags in examples Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 19/46] chelsio/chtls: fix panic during unload reload chtls Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 20/46] ibmvnic: Ensure that SCRQ entry reads are correctly ordered Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 21/46] ibmvnic: Fix TX completion error handling Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 22/46] tipc: fix incompatible mtu of transmission Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 23/46] inet_ecn: Fix endianness of checksum update when setting ECT(1) Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 24/46] geneve: pull IP header before ECN decapsulation Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 25/46] net: ip6_gre: set dev->hard_header_len when using header_ops Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 26/46] dpaa_eth: copy timestamp fields to new skb in A-050385 workaround Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 27/46] net/x25: prevent a couple of overflows Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 28/46] cxgb3: fix error return code in t3_sge_alloc_qset() Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 29/46] net: pasemi: fix error return code in pasemi_mac_open() Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 30/46] vxlan: fix error return code in __vxlan_dev_create() Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 31/46] chelsio/chtls: fix a double free in chtls_setkey() Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 32/46] net: mvpp2: Fix error return code in mvpp2_open() Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 33/46] net: skbuff: ensure LSE is pullable before decrementing the MPLS ttl Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 34/46] net: openvswitch: ensure LSE is pullable before reading it Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 35/46] net/sched: act_mpls: " Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 36/46] net/mlx5: DR, Proper handling of unsupported Connect-X6DX SW steering Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 37/46] net/mlx5: Fix wrong address reclaim when command interface is down Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 38/46] net: mlx5e: fix fs_tcp.c build when IPV6 is not enabled Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 39/46] ALSA: usb-audio: US16x08: fix value count for level meters Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 40/46] Input: xpad - support Ardwiino Controllers Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 41/46] Input: i8042 - add ByteSpeed touchpad to noloop table Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 42/46] Input: atmel_mxt_ts - fix lost interrupts Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 43/46] tracing: Fix alignment of static buffer Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 44/46] tracing: Remove WARN_ON in start_thread() Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 45/46] uapi: fix statx attribute value overlap for DAX & MOUNT_ROOT Greg Kroah-Hartman
2020-12-06 11:17 ` [PATCH 5.9 46/46] drm/i915/gt: Fixup tgl mocs for PTE tracking Greg Kroah-Hartman
2020-12-06 21:08 ` [PATCH 5.9 00/46] 5.9.13-rc1 review Jeffrin Jose T
2020-12-07  7:17 ` Naresh Kamboju
2020-12-08  9:42   ` Greg Kroah-Hartman
2020-12-07 15:04 ` Jon Hunter
2020-12-08  9:43   ` Greg Kroah-Hartman
2020-12-07 15:55 ` Guenter Roeck
2020-12-08  9:43   ` Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201206111557.197220214@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=Jason@zx2c4.com \
    --cc=eyal.birger@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=willemb@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.