[PATCH net v2] net/sched: act_ife: Only operate on Ethernet frames

act_ife encapsulates/decapsulates the original Ethernet header and uses
skb->dev->hard_header_len as the length of that header. That is only
correct for Ethernet devices: on a device where hard_header_len does not
match the L2 header that was actually pulled (PPP reports PPP_HDRLEN
while nothing is stripped on ingress), the ingress skb_push()/skb_pull()
use the wrong length and can hit skb_under_panic when headroom is tight.

IFE is Ethernet-only by design - it builds an outer ethhdr, rewrites
h_source/h_dest/h_proto, and calls eth_type_trans() on decode - so
instead of trying to make the offsets work for arbitrary link types,
simply drop packets that do not carry an Ethernet header.

Checking skb->dev->type alone is not enough. We have to cater for a
corner case where mirred can redirect an skb from a non-Ethernet device
to an Ethernet one, and skb->dev then says nothing about the framing the
skb actually has: an skb redirected from ppp0 reaches the target's ingress
hook with mac_len 0 and no Ethernet header at all. So at ingress also
require mac_len to be ETH_HLEN. On egress mac_len is not maintained, so
the device type is all we have; a bogus redirect there yields a malformed
frame rather than an out-of-bounds push, and it would be malformed with or
without IFE.

That corner case is not theoretical - redirecting from ppp0 into a veth
that has an ife encode action on its ingress hook panics without this
patch:

  skbuff: skb_under_panic: len:98 put:14 head:ffff88800e410000
          data:ffff88800e40fff5 tail:0x57 end:0x640 dev:veth3
  kernel BUG at net/core/skbuff.c:214!
  Call Trace:
   skb_push (net/core/skbuff.c:224 net/core/skbuff.c:2657)
   tcf_ife_act (net/sched/act_ife.c:829 net/sched/act_ife.c:874)
   tc_run (net/core/dev.c:4463)
   netif_receive_skb (net/core/dev.c:6463 net/core/dev.c:6522)
   tcf_mirred_to_dev (net/sched/act_mirred.c:248 net/sched/act_mirred.c:328)
   tcf_mirred_act (net/sched/act_mirred.c:489)
   tc_run (net/core/dev.c:4463)
   process_backlog (net/core/dev.c:6728)

With Ethernet framing guaranteed, use ETH_HLEN instead of
hard_header_len.

Fixes: 295a6e06d21e ("net/sched: act_ife: Change to use ife module")
Reported-by: [email protected]
Acked-by: Jamal Hadi Salim <[email protected]>
Signed-off-by: Victor Nogueira <[email protected]>
---
v1 -> v2:
  - retitled; v1 was "net/sched: act_ife: use mac_len for ingress header
    offset".
  - use ETH_HLEN throughout rather than hard_header_len/mac_len
  - Reject skbs mirred-redirected here from non-Ethernet devicec - Jakub
  - Also reject skbs where skb->dev is Ethernet but the framing is not
v1: https://lore.kernel.org/netdev/[email protected]/
---
 net/ife/ife.c       | 14 +++++++-------
 net/sched/act_ife.c | 32 ++++++++++++++++++++++++++++----
 2 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/net/ife/ife.c b/net/ife/ife.c
index 7a75947a31e3..2ddf725d3389 100644
--- a/net/ife/ife.c
+++ b/net/ife/ife.c
@@ -37,7 +37,7 @@ void *ife_encode(struct sk_buff *skb, u16 metalen)
 	 * where ORIGDATA = original ethernet header ...
 	 */
 	int hdrm = metalen + IFE_METAHDRLEN;
-	int total_push = hdrm + skb->dev->hard_header_len;
+	int total_push = hdrm + ETH_HLEN;
 	struct ifeheadr *ifehdr;
 	struct ethhdr *iethh;	/* inner ether header */
 	int skboff = 0;
@@ -50,9 +50,9 @@ void *ife_encode(struct sk_buff *skb, u16 metalen)
 	iethh = (struct ethhdr *) skb->data;
 
 	__skb_push(skb, total_push);
-	memcpy(skb->data, iethh, skb->dev->hard_header_len);
+	memcpy(skb->data, iethh, ETH_HLEN);
 	skb_reset_mac_header(skb);
-	skboff += skb->dev->hard_header_len;
+	skboff += ETH_HLEN;
 
 	/* total metadata length */
 	ifehdr = (struct ifeheadr *) (skb->data + skboff);
@@ -69,12 +69,12 @@ void *ife_decode(struct sk_buff *skb, u16 *metalen)
 	int total_pull;
 	u16 ifehdrln;
 
-	if (!pskb_may_pull(skb, skb->dev->hard_header_len + IFE_METAHDRLEN))
+	if (!pskb_may_pull(skb, ETH_HLEN + IFE_METAHDRLEN))
 		return NULL;
 
-	ifehdr = (struct ifeheadr *) (skb->data + skb->dev->hard_header_len);
+	ifehdr = (struct ifeheadr *)(skb->data + ETH_HLEN);
 	ifehdrln = ntohs(ifehdr->metalen);
-	total_pull = skb->dev->hard_header_len + ifehdrln;
+	total_pull = ETH_HLEN + ifehdrln;
 
 	if (unlikely(ifehdrln < 2))
 		return NULL;
@@ -82,7 +82,7 @@ void *ife_decode(struct sk_buff *skb, u16 *metalen)
 	if (unlikely(!pskb_may_pull(skb, total_pull + ETH_HLEN)))
 		return NULL;
 
-	ifehdr = (struct ifeheadr *)(skb->data + skb->dev->hard_header_len);
+	ifehdr = (struct ifeheadr *)(skb->data + ETH_HLEN);
 	skb_set_mac_header(skb, total_pull);
 	__skb_pull(skb, total_pull);
 	*metalen = ifehdrln - IFE_METAHDRLEN;
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index ff2b16e35b9b..9cea71fc1db3 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -28,6 +28,7 @@
 #include <uapi/linux/tc_act/tc_ife.h>
 #include <net/tc_act/tc_ife.h>
 #include <linux/etherdevice.h>
+#include <linux/if_arp.h>
 #include <net/ife.h>
 #include <net/tc_wrapper.h>
 
@@ -723,7 +724,7 @@ static int tcf_ife_decode(struct sk_buff *skb, const struct tc_action *a,
 	tcf_lastuse_update(&ife->tcf_tm);
 
 	if (skb_at_tc_ingress(skb))
-		skb_push(skb, skb->dev->hard_header_len);
+		skb_push(skb, ETH_HLEN);
 
 	tlv_data = ife_decode(skb, &metalen);
 	if (unlikely(!tlv_data)) {
@@ -795,7 +796,7 @@ static int tcf_ife_encode(struct sk_buff *skb, const struct tc_action *a,
 	   where ORIGDATA = original ethernet header ...
 	 */
 	u16 metalen = ife_get_sz(skb, p);
-	int hdrm = metalen + skb->dev->hard_header_len + IFE_METAHDRLEN;
+	int hdrm = metalen + ETH_HLEN + IFE_METAHDRLEN;
 	unsigned int skboff = 0;
 	int new_len = skb->len + hdrm;
 	bool exceed_mtu = false;
@@ -826,7 +827,7 @@ static int tcf_ife_encode(struct sk_buff *skb, const struct tc_action *a,
 	}
 
 	if (skb_at_tc_ingress(skb))
-		skb_push(skb, skb->dev->hard_header_len);
+		skb_push(skb, ETH_HLEN);
 
 	ife_meta = ife_encode(skb, metalen);
 	if (!ife_meta)
@@ -856,11 +857,27 @@ static int tcf_ife_encode(struct sk_buff *skb, const struct tc_action *a,
 	oethh->h_proto = htons(p->eth_type);
 
 	if (skb_at_tc_ingress(skb))
-		skb_pull(skb, skb->dev->hard_header_len);
+		skb_pull(skb, ETH_HLEN);
 
 	return action;
 }
 
+/* IFE encapsulates the original Ethernet header and, on decode, expects to
+ * find one, so it can only ever work on skbs that carry one. Loopback carries
+ * Ethernet header as well, so it qualifies here.
+ * At ingress, also verify that the L2 header about to be pushed back really
+ * is an Ethernet header because the skb could've been redirected with mirred
+ * from a non-Ethernet device.
+ */
+static bool tcf_ife_is_eth_skb(const struct sk_buff *skb)
+{
+	if (skb->dev->type != ARPHRD_ETHER &&
+	    skb->dev->type != ARPHRD_LOOPBACK)
+		return false;
+
+	return !skb_at_tc_ingress(skb) || skb->mac_len == ETH_HLEN;
+}
+
 TC_INDIRECT_SCOPE int tcf_ife_act(struct sk_buff *skb,
 				  const struct tc_action *a,
 				  struct tcf_result *res)
@@ -869,6 +886,13 @@ TC_INDIRECT_SCOPE int tcf_ife_act(struct sk_buff *skb,
 	struct tcf_ife_params *p;
 	int ret;
 
+	if (unlikely(!tcf_ife_is_eth_skb(skb))) {
+		bstats_update(this_cpu_ptr(ife->common.cpu_bstats), skb);
+		tcf_lastuse_update(&ife->tcf_tm);
+		qstats_cpu_drop_inc(ife->common.cpu_qstats);
+		return TC_ACT_SHOT;
+	}
+
 	p = rcu_dereference_bh(ife->params);
 	if (p->flags & IFE_ENCODE) {
 		ret = tcf_ife_encode(skb, a, res, p);
-- 
2.43.0