[PATCH net 0/2] ieee802154: 6lowpan: fix transmit address handling

[PATCH net 0/2] ieee802154: 6lowpan: fix transmit address handling

[Eric Dumazet]

SEFCOM lab reported a warning in lowpan_compress_addr_64(),
triggered by an AF_PACKET socket sending on a lowpan device.

lowpan_header_create() stores the 802.15.4 addresses in the skb
headroom, where lowpan_xmit() cannot tell if they are valid.
It also reads the IPv6 header before AF_PACKET SOCK_DGRAM sockets
have copied it.

Patch 1 moves the neighbour short address lookup to lowpan_xmit().

Patch 2 pushes the destination address as an 8 byte pseudo header,
like IPoIB, and lets lowpan_xmit() compute the 802.15.4 addresses.

Eric Dumazet (2):
  ieee802154: 6lowpan: look up the neighbour short address in
    lowpan_xmit()
  6lowpan: fix warning in lowpan_compress_addr_64()

 net/ieee802154/6lowpan/6lowpan_i.h |   5 ++
 net/ieee802154/6lowpan/core.c      |   4 +-
 net/ieee802154/6lowpan/tx.c        | 128 ++++++++++++++---------------
 3 files changed, 70 insertions(+), 67 deletions(-)

-- 
2.56.0.rc1.315.gc6ed9934b7-goog

[PATCH net 1/2] ieee802154: 6lowpan: look up the neighbour short address in lowpan_xmit()

[Eric Dumazet]

lowpan_header_create() uses ipv6_hdr(skb)->daddr to find the
neighbour, and to use its 802.15.4 short address if it has one.

The IPv6 header is not there yet when lowpan_header_create() is
called for AF_PACKET SOCK_DGRAM sockets: packet_snd() copies the
payload after dev_hard_header(), and tpacket_fill_skb() attaches it
as page fragments after dev_hard_header(). The lookup then reads
uninitialized memory, possibly past the end of the linear data.

Move the lookup to lowpan_header(), called from lowpan_xmit().

lowpan_xmit() now makes sure the IPv6 header is in the linear part
of the skb, and resets the network header, since AF_PACKET SOCK_RAW
sockets set it after dev->hard_header_len bytes.
lowpan_header_compress() expects the IPv6 header at skb->data.

Fixes: eab560e58208 ("6lowpan: add support for 802.15.4 short addr handling")
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
Cc: Alexander Aring <alex.aring@gmail.com>
---
 net/ieee802154/6lowpan/tx.c | 63 ++++++++++++++++++++++---------------
 1 file changed, 37 insertions(+), 26 deletions(-)

diff --git a/net/ieee802154/6lowpan/tx.c b/net/ieee802154/6lowpan/tx.c
index 4df76ff50699ede5c187c9cca6f0cc10b19d2123..2d83a810e610845dc0476ed3f12a6479841c8819 100644
--- a/net/ieee802154/6lowpan/tx.c
+++ b/net/ieee802154/6lowpan/tx.c
@@ -36,9 +36,6 @@ int lowpan_header_create(struct sk_buff *skb, struct net_device *ldev,
 {
 	struct wpan_dev *wpan_dev = lowpan_802154_dev(ldev)->wdev->ieee802154_ptr;
 	struct lowpan_addr_info *info = lowpan_skb_priv(skb);
-	struct lowpan_802154_neigh *llneigh = NULL;
-	const struct ipv6hdr *hdr = ipv6_hdr(skb);
-	struct neighbour *n;
 
 	if (!daddr)
 		return -EINVAL;
@@ -57,28 +54,12 @@ int lowpan_header_create(struct sk_buff *skb, struct net_device *ldev,
 		info->daddr.short_addr = cpu_to_le16(IEEE802154_ADDR_BROADCAST);
 		info->daddr.mode = IEEE802154_ADDR_SHORT;
 	} else {
-		__le16 short_addr = cpu_to_le16(IEEE802154_ADDR_SHORT_UNSPEC);
-
-		n = neigh_lookup(&nd_tbl, &hdr->daddr, ldev);
-		if (n) {
-			llneigh = lowpan_802154_neigh(neighbour_priv(n));
-			read_lock_bh(&n->lock);
-			short_addr = llneigh->short_addr;
-			read_unlock_bh(&n->lock);
-		}
-
-		if (llneigh &&
-		    lowpan_802154_is_valid_src_short_addr(short_addr)) {
-			info->daddr.short_addr = short_addr;
-			info->daddr.mode = IEEE802154_ADDR_SHORT;
-		} else {
-			info->daddr.mode = IEEE802154_ADDR_LONG;
-			ieee802154_be64_to_le64(&info->daddr.extended_addr,
-						daddr);
-		}
-
-		if (n)
-			neigh_release(n);
+		/* The IPv6 header might not be there yet (AF_PACKET).
+		 * lowpan_header() will use the neighbour short address,
+		 * if there is one.
+		 */
+		info->daddr.mode = IEEE802154_ADDR_LONG;
+		ieee802154_be64_to_le64(&info->daddr.extended_addr, daddr);
 	}
 
 	if (!saddr) {
@@ -221,6 +202,31 @@ lowpan_xmit_fragmented(struct sk_buff *skb, struct net_device *ldev,
 	return rc;
 }
 
+/* Use the short address of the IPv6 destination, if the neighbour has one. */
+static void lowpan_neigh_short_addr(const struct sk_buff *skb,
+				    struct net_device *ldev,
+				    struct ieee802154_addr *daddr)
+{
+	struct lowpan_802154_neigh *llneigh;
+	struct neighbour *n;
+	__le16 short_addr;
+
+	n = neigh_lookup(&nd_tbl, &ipv6_hdr(skb)->daddr, ldev);
+	if (!n)
+		return;
+
+	llneigh = lowpan_802154_neigh(neighbour_priv(n));
+	read_lock_bh(&n->lock);
+	short_addr = llneigh->short_addr;
+	read_unlock_bh(&n->lock);
+	neigh_release(n);
+
+	if (lowpan_802154_is_valid_src_short_addr(short_addr)) {
+		daddr->short_addr = short_addr;
+		daddr->mode = IEEE802154_ADDR_SHORT;
+	}
+}
+
 static int lowpan_header(struct sk_buff *skb, struct net_device *ldev,
 			 u16 *dgram_size, u16 *dgram_offset)
 {
@@ -230,6 +236,9 @@ static int lowpan_header(struct sk_buff *skb, struct net_device *ldev,
 
 	memcpy(&info, lowpan_skb_priv(skb), sizeof(info));
 
+	if (info.daddr.mode == IEEE802154_ADDR_LONG)
+		lowpan_neigh_short_addr(skb, ldev, &info.daddr);
+
 	*dgram_size = skb->len;
 	lowpan_header_compress(skb, ldev, &info.daddr, &info.saddr);
 	/* dgram_offset = (saved bytes after compression) + lowpan header len */
@@ -255,10 +264,12 @@ netdev_tx_t lowpan_xmit(struct sk_buff *skb, struct net_device *ldev)
 
 	pr_debug("package xmit\n");
 
-	if (skb->protocol != htons(ETH_P_IPV6)) {
+	if (skb->protocol != htons(ETH_P_IPV6) ||
+	    !pskb_may_pull(skb, sizeof(struct ipv6hdr))) {
 		kfree_skb(skb);
 		return NET_XMIT_DROP;
 	}
+	skb_reset_network_header(skb);
 
 	WARN_ON_ONCE(skb->len > IPV6_MIN_MTU);
 
-- 
2.56.0.rc1.315.gc6ed9934b7-goog

[PATCH net 2/2] 6lowpan: fix warning in lowpan_compress_addr_64()

[Eric Dumazet]

SEFCOM lab reported a warning in lowpan_compress_addr_64():

WARNING: net/6lowpan/iphc.c:937 at lowpan_iphc_compress_802154_lladdr net/6lowpan/iphc.c:937 [inline]
WARNING: net/6lowpan/iphc.c:937 at lowpan_compress_addr_64+0x4be/0x9c0 net/6lowpan/iphc.c:952
Call Trace:
 lowpan_compress_addr_64+0x4be/0x9c0 net/6lowpan/iphc.c:952
 lowpan_header_compress+0xeef/0x1ef0 net/6lowpan/iphc.c:1242
 lowpan_header net/ieee802154/6lowpan/tx.c:234 [inline]
 lowpan_xmit+0x4c6/0x1420 net/ieee802154/6lowpan/tx.c:282
 dev_hard_start_xmit+0x23b/0x620 net/core/dev.c:3904
 __dev_queue_xmit+0x11e7/0x3250 net/core/dev.c:4870
 packet_snd net/packet/af_packet.c:3082 [inline]
 packet_sendmsg+0x3d9b/0x5150 net/packet/af_packet.c:3114

lowpan_header_create() stores the 802.15.4 addresses in the skb
headroom, without changing skb->data, and lowpan_xmit() reads them
from there. But lowpan_xmit() cannot know if this was done:

- AF_PACKET SOCK_RAW sockets do not call dev_hard_header().
- GSO segments get a new headroom: skb_segment() only copies
  data starting at the mac header.

lowpan_xmit() then uses uninitialized memory, and can call
lowpan_header_compress() with invalid address modes.

Fix this by pushing the destination address as a pseudo header,
like IPoIB in ipoib_hard_header(). The pseudo header is the 8 byte
EUI-64 given to dev_hard_header(), as in sockaddr_ll.sll_addr,
so its layout does not depend on the architecture.

lowpan_xmit() pulls it, then lowpan_header() computes the 802.15.4
addresses like lowpan_header_create() did. Like IPoIB, the saddr
argument is ignored: the IPv6 stack and AF_PACKET pass NULL, and
the wpan device address is used.

Set hard_header_len to the pseudo header size, so that AF_PACKET
SOCK_RAW sockets set the network header at the right place.
lowpan_xmit() now checks the minimum length itself.

Also return -EINVAL from lowpan_header_create() for non IPv6
packets, like net/bluetooth/6lowpan.c, instead of returning 0
without a pseudo header. lowpan_xmit() drops them anyway.

IPv6 stack and AF_PACKET SOCK_DGRAM users are not affected.
AF_PACKET SOCK_RAW senders now have to put the destination address
in front of the IPv6 header, and SOCK_RAW packet taps see it
on transmit. Received packets are unchanged: their framing for
SOCK_RAW taps already depends on the 6LoWPAN dispatch type.
tcpdump is not affected, libpcap uses cooked mode for ARPHRD_6LOWPAN.

Fixes: eab560e58208 ("6lowpan: add support for 802.15.4 short addr handling")
Reported-by: Farhad Alemi <falemi@asu.edu>
Closes: https://lore.kernel.org/netdev/CA+0ovChS18paCd=ZE-7j_M-JtFF-N6+A75deKUqJ0Yy7ux=h2Q@mail.gmail.com/
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
Cc: Alexander Aring <alex.aring@gmail.com>
---
 net/ieee802154/6lowpan/6lowpan_i.h |  5 ++
 net/ieee802154/6lowpan/core.c      |  4 +-
 net/ieee802154/6lowpan/tx.c        | 81 +++++++++++++-----------------
 3 files changed, 41 insertions(+), 49 deletions(-)

diff --git a/net/ieee802154/6lowpan/6lowpan_i.h b/net/ieee802154/6lowpan/6lowpan_i.h
index 44a7e16bf3b5e14c03b99a806145203fc2a4af01..ccc49d10983318b1370a659ff61649bf339fb8c7 100644
--- a/net/ieee802154/6lowpan/6lowpan_i.h
+++ b/net/ieee802154/6lowpan/6lowpan_i.h
@@ -30,6 +30,11 @@ struct lowpan_frag_queue {
 	struct inet_frag_queue	q;
 };
 
+/* lowpan_header_create() pushes the destination address (an EUI-64, as in
+ * sockaddr_ll.sll_addr) in front of the IPv6 header, lowpan_xmit() pulls it.
+ */
+#define LOWPAN_PSEUDO_HDR_LEN	EUI64_ADDR_LEN
+
 int lowpan_frag_rcv(struct sk_buff *skb, const u8 frag_type);
 void lowpan_net_frag_exit(void);
 int lowpan_net_frag_init(void);
diff --git a/net/ieee802154/6lowpan/core.c b/net/ieee802154/6lowpan/core.c
index 6a8d6852cb93057305a1a6c5398a3c072bde9f0f..c70b5e414fdf2a68013ded7b0d1d51ba3e3f55de 100644
--- a/net/ieee802154/6lowpan/core.c
+++ b/net/ieee802154/6lowpan/core.c
@@ -109,8 +109,8 @@ static const struct net_device_ops lowpan_netdev_ops = {
 static void lowpan_setup(struct net_device *ldev)
 {
 	memset(ldev->broadcast, 0xff, IEEE802154_ADDR_LEN);
-	/* We need an ipv6hdr as minimum len when calling xmit */
-	ldev->hard_header_len	= sizeof(struct ipv6hdr);
+	/* lowpan_header_create() pushes the destination address */
+	ldev->hard_header_len	= LOWPAN_PSEUDO_HDR_LEN;
 	ldev->flags		= IFF_BROADCAST | IFF_MULTICAST;
 	ldev->priv_flags	|= IFF_NO_QUEUE;
 
diff --git a/net/ieee802154/6lowpan/tx.c b/net/ieee802154/6lowpan/tx.c
index 2d83a810e610845dc0476ed3f12a6479841c8819..10ce2928fcca1a9fd535e8e35f2cac3f792e82f6 100644
--- a/net/ieee802154/6lowpan/tx.c
+++ b/net/ieee802154/6lowpan/tx.c
@@ -15,17 +15,13 @@ struct lowpan_addr_info {
 	struct ieee802154_addr saddr;
 };
 
-static inline struct
-lowpan_addr_info *lowpan_skb_priv(const struct sk_buff *skb)
-{
-	WARN_ON_ONCE(skb_headroom(skb) < sizeof(struct lowpan_addr_info));
-	return (struct lowpan_addr_info *)(skb->data -
-			sizeof(struct lowpan_addr_info));
-}
-
 /* This callback will be called from AF_PACKET and IPv6 stack, the AF_PACKET
  * sockets gives an 8 byte array for addresses only!
  *
+ * Like IPoIB, the destination address is pushed as a pseudo header, which
+ * AF_PACKET SOCK_RAW senders have to provide. lowpan_xmit() pulls it, and
+ * always uses the wpan device address as source address.
+ *
  * TODO I think AF_PACKET DGRAM (sending/receiving) RAW (sending) makes no
  * sense here. We should disable it, the right use-case would be AF_INET6
  * RAW/DGRAM sockets.
@@ -34,9 +30,6 @@ int lowpan_header_create(struct sk_buff *skb, struct net_device *ldev,
 			 unsigned short type, const void *daddr,
 			 const void *saddr, unsigned int len)
 {
-	struct wpan_dev *wpan_dev = lowpan_802154_dev(ldev)->wdev->ieee802154_ptr;
-	struct lowpan_addr_info *info = lowpan_skb_priv(skb);
-
 	if (!daddr)
 		return -EINVAL;
 
@@ -44,38 +37,11 @@ int lowpan_header_create(struct sk_buff *skb, struct net_device *ldev,
 	 * if this package isn't ipv6 one, where should it be routed?
 	 */
 	if (type != ETH_P_IPV6)
-		return 0;
-
-	/* intra-pan communication */
-	info->saddr.pan_id = wpan_dev->pan_id;
-	info->daddr.pan_id = info->saddr.pan_id;
-
-	if (!memcmp(daddr, ldev->broadcast, EUI64_ADDR_LEN)) {
-		info->daddr.short_addr = cpu_to_le16(IEEE802154_ADDR_BROADCAST);
-		info->daddr.mode = IEEE802154_ADDR_SHORT;
-	} else {
-		/* The IPv6 header might not be there yet (AF_PACKET).
-		 * lowpan_header() will use the neighbour short address,
-		 * if there is one.
-		 */
-		info->daddr.mode = IEEE802154_ADDR_LONG;
-		ieee802154_be64_to_le64(&info->daddr.extended_addr, daddr);
-	}
-
-	if (!saddr) {
-		if (lowpan_802154_is_valid_src_short_addr(wpan_dev->short_addr)) {
-			info->saddr.mode = IEEE802154_ADDR_SHORT;
-			info->saddr.short_addr = wpan_dev->short_addr;
-		} else {
-			info->saddr.mode = IEEE802154_ADDR_LONG;
-			info->saddr.extended_addr = wpan_dev->extended_addr;
-		}
-	} else {
-		info->saddr.mode = IEEE802154_ADDR_LONG;
-		ieee802154_be64_to_le64(&info->saddr.extended_addr, saddr);
-	}
+		return -EINVAL;
 
-	return 0;
+	memcpy(skb_push(skb, LOWPAN_PSEUDO_HDR_LEN), daddr,
+	       LOWPAN_PSEUDO_HDR_LEN);
+	return LOWPAN_PSEUDO_HDR_LEN;
 }
 
 static struct sk_buff*
@@ -228,16 +194,32 @@ static void lowpan_neigh_short_addr(const struct sk_buff *skb,
 }
 
 static int lowpan_header(struct sk_buff *skb, struct net_device *ldev,
-			 u16 *dgram_size, u16 *dgram_offset)
+			 const u8 *daddr, u16 *dgram_size, u16 *dgram_offset)
 {
 	struct wpan_dev *wpan_dev = lowpan_802154_dev(ldev)->wdev->ieee802154_ptr;
 	struct ieee802154_mac_cb *cb = mac_cb_init(skb);
 	struct lowpan_addr_info info;
 
-	memcpy(&info, lowpan_skb_priv(skb), sizeof(info));
+	/* intra-pan communication */
+	info.saddr.pan_id = wpan_dev->pan_id;
+	info.daddr.pan_id = info.saddr.pan_id;
 
-	if (info.daddr.mode == IEEE802154_ADDR_LONG)
+	if (!memcmp(daddr, ldev->broadcast, EUI64_ADDR_LEN)) {
+		info.daddr.short_addr = cpu_to_le16(IEEE802154_ADDR_BROADCAST);
+		info.daddr.mode = IEEE802154_ADDR_SHORT;
+	} else {
+		info.daddr.mode = IEEE802154_ADDR_LONG;
+		ieee802154_be64_to_le64(&info.daddr.extended_addr, daddr);
 		lowpan_neigh_short_addr(skb, ldev, &info.daddr);
+	}
+
+	if (lowpan_802154_is_valid_src_short_addr(wpan_dev->short_addr)) {
+		info.saddr.mode = IEEE802154_ADDR_SHORT;
+		info.saddr.short_addr = wpan_dev->short_addr;
+	} else {
+		info.saddr.mode = IEEE802154_ADDR_LONG;
+		info.saddr.extended_addr = wpan_dev->extended_addr;
+	}
 
 	*dgram_size = skb->len;
 	lowpan_header_compress(skb, ldev, &info.daddr, &info.saddr);
@@ -258,6 +240,7 @@ static int lowpan_header(struct sk_buff *skb, struct net_device *ldev,
 
 netdev_tx_t lowpan_xmit(struct sk_buff *skb, struct net_device *ldev)
 {
+	u8 daddr[LOWPAN_PSEUDO_HDR_LEN];
 	struct ieee802154_hdr wpan_hdr;
 	int max_single, ret;
 	u16 dgram_size, dgram_offset;
@@ -265,10 +248,14 @@ netdev_tx_t lowpan_xmit(struct sk_buff *skb, struct net_device *ldev)
 	pr_debug("package xmit\n");
 
 	if (skb->protocol != htons(ETH_P_IPV6) ||
-	    !pskb_may_pull(skb, sizeof(struct ipv6hdr))) {
+	    !pskb_may_pull(skb, sizeof(daddr) + sizeof(struct ipv6hdr))) {
 		kfree_skb(skb);
 		return NET_XMIT_DROP;
 	}
+
+	/* Destination address pushed by lowpan_header_create() */
+	memcpy(daddr, skb->data, sizeof(daddr));
+	__skb_pull(skb, sizeof(daddr));
 	skb_reset_network_header(skb);
 
 	WARN_ON_ONCE(skb->len > IPV6_MIN_MTU);
@@ -295,7 +282,7 @@ netdev_tx_t lowpan_xmit(struct sk_buff *skb, struct net_device *ldev)
 			return NET_XMIT_DROP;
 	}
 
-	ret = lowpan_header(skb, ldev, &dgram_size, &dgram_offset);
+	ret = lowpan_header(skb, ldev, daddr, &dgram_size, &dgram_offset);
 	if (ret < 0) {
 		kfree_skb(skb);
 		return NET_XMIT_DROP;
-- 
2.56.0.rc1.315.gc6ed9934b7-goog

Re: [PATCH net 1/2] ieee802154: 6lowpan: look up the neighbour short address in lowpan_xmit()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 (1) · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] At this commit, lowpan_xmit() (net/ieee802154/6lowpan/tx.c:272)
  resets skb->network_header to skb->data but leaves…

Pre-existing issues:
- [Medium] At this commit, AF_PACKET SOCK_RAW senders reach lowpan_xmit()
  without dev_hard_header() ever being called.

--

ieee802154: 6lowpan: look up the neighbour short address in lowpan_xmit()

This commit moves the neighbour short address lookup from
lowpan_header_create() to lowpan_header(). The lookup then runs after the
IPv6 header has been copied into the skb. lowpan_xmit() now pulls the IPv6
header into the linear area and resets the network header to skb->data.

> diff --git a/net/ieee802154/6lowpan/tx.c b/net/ieee802154/6lowpan/tx.c
> index 4df76ff50699..2d83a810e610 100644
> --- a/net/ieee802154/6lowpan/tx.c
> +++ b/net/ieee802154/6lowpan/tx.c

[ ... ]

> @@ -230,6 +236,9 @@ static int lowpan_header(struct sk_buff *skb, struct net_device *ldev,
>  
>  	memcpy(&info, lowpan_skb_priv(skb), sizeof(info));
>  
> +	if (info.daddr.mode == IEEE802154_ADDR_LONG)
> +		lowpan_neigh_short_addr(skb, ldev, &info.daddr);

[Severity: Medium]
This issue was not introduced by this patch, but is info.daddr.mode ever
initialized here for AF_PACKET SOCK_RAW senders?

packet_snd() and tpacket_fill_skb() only call dev_hard_header() for
SOCK_DGRAM:

net/packet/af_packet.c:packet_snd() {
    ...
	if (sock->type == SOCK_DGRAM) {
		offset = dev_hard_header(skb, dev, ntohs(proto), addr, NULL, len);
    ...
}

So for SOCK_RAW, lowpan_header_create() never writes the struct
lowpan_addr_info at lowpan_skb_priv(skb). That headroom is memory from
sock_alloc_send_pskb() that has not been zeroed.

The memcpy() above copies it into info, and the new check branches on the
uninitialized daddr.mode. As before this patch, the rest of the
uninitialized daddr/saddr still reaches lowpan_header_compress(), the
ackreq decision and wpan_dev_hard_header().

The next patch in the series, "6lowpan: fix warning in
lowpan_compress_addr_64()", appears to fix this. It removes
lowpan_skb_priv() and builds daddr/saddr from the pushed pseudo header and
wpan_dev.

[ ... ]

> @@ -255,10 +264,12 @@ netdev_tx_t lowpan_xmit(struct sk_buff *skb, struct net_device *ldev)
>  
>  	pr_debug("package xmit\n");
>  
> -	if (skb->protocol != htons(ETH_P_IPV6)) {
> +	if (skb->protocol != htons(ETH_P_IPV6) ||
> +	    !pskb_may_pull(skb, sizeof(struct ipv6hdr))) {
>  		kfree_skb(skb);
>  		return NET_XMIT_DROP;
>  	}
> +	skb_reset_network_header(skb);

[Severity: High]
This resets only the network header. Can skb->transport_header go stale
for AF_PACKET SOCK_RAW senders as a result?

At this commit ldev->hard_header_len is sizeof(struct ipv6hdr). For
SOCK_RAW with len >= 80, packet_snd() therefore leaves the network header
at data + 40:

	} else if (reserve) {
		skb_reserve(skb, -reserve);
		if (len < reserve + sizeof(struct ipv6hdr) &&
		    dev->min_header_len != hard_header_len)
			skb_reset_network_header(skb);
	}

packet_parse_headers()->skb_probe_transport_header() then parses bytes
40..79 as the IPv6 header. transport_header ends up around data + 80, or
stays unset (0xffff) if parsing fails. tpacket_fill_skb() on the TX ring
path also places the network header hard_header_len bytes after skb->data.

After this reset the network header is at data + 0. If byte 6 (nexthdr) is
IPPROTO_UDP, lowpan_header_compress()->lowpan_nhc_do_compression() only
fixes the transport header when the two offsets are equal:

net/6lowpan/nhc.c:lowpan_nhc_do_compression() {
    ...
	if (skb->transport_header == skb->network_header)
		skb_set_transport_header(skb, sizeof(struct ipv6hdr));
    ...
}

That check no longer holds, so udp_compress() does:

	const struct udphdr *uh = udp_hdr(skb);

at the stale offset. That is bytes 80..87, or skb->head + 0xffff if the
transport header was never set.

Can this read out of bounds, and can the bytes it reads go into the
transmitted 802.15.4 frame as the UDP ports and checksum?

Only this intermediate commit seems to be affected. The next patch in the
series, "6lowpan: fix warning in lowpan_compress_addr_64()", sets
hard_header_len to LOWPAN_PSEUDO_HDR_LEN. It also calls __skb_pull() on the
pseudo header before skb_reset_network_header(). The reset network header
then matches where AF_PACKET put it.

Could the two changes be reordered or combined so that this commit is safe
to bisect?

A separate part of this was not introduced by this patch and is still
present at the end of the series. pskb_may_pull() only guarantees the IPv6
header in the linear area (8 + 40 bytes after the next patch). If nexthdr
is UDP and the payload is shorter than 8 bytes, can udp_compress() read up
to 8 bytes past skb->tail?

>  
>  	WARN_ON_ONCE(skb->len > IPV6_MIN_MTU);
>

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260928183753.3550210-1-edumazet%40kernel.org

Re: [PATCH net 1/2] ieee802154: 6lowpan: look up the neighbour short address in lowpan_xmit()

[Eric Dumazet]

On Fri, Oct 2, 2026 at 8:39 AM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
>
> Critical: 0 · High: 1 · Medium: 0 (1) · Low: 0
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [High] At this commit, lowpan_xmit() (net/ieee802154/6lowpan/tx.c:272)
>   resets skb->network_header to skb->data but leaves…
>

Right, at this commit hard_header_len is still 40, so the
skb_reset_network_header() leaves the transport header probed for
AF_PACKET SOCK_RAW at the old offset. I will move the reset to patch 2,
where hard_header_len becomes 8 and the AF_PACKET offsets already match.

The Medium is the bug fixed by patch 2.

The truncated UDP header read in udp_compress() predates this series
and is also reachable from AF_INET6 raw sockets, I will send a separate fix.

pw-bot: cr

> Pre-existing issues:
> - [Medium] At this commit, AF_PACKET SOCK_RAW senders reach lowpan_xmit()
>   without dev_hard_header() ever being called.
>
> --
>
> ieee802154: 6lowpan: look up the neighbour short address in lowpan_xmit()
>
> This commit moves the neighbour short address lookup from
> lowpan_header_create() to lowpan_header(). The lookup then runs after the
> IPv6 header has been copied into the skb. lowpan_xmit() now pulls the IPv6
> header into the linear area and resets the network header to skb->data.
>
> > diff --git a/net/ieee802154/6lowpan/tx.c b/net/ieee802154/6lowpan/tx.c
> > index 4df76ff50699..2d83a810e610 100644
> > --- a/net/ieee802154/6lowpan/tx.c
> > +++ b/net/ieee802154/6lowpan/tx.c
>
> [ ... ]
>
> > @@ -230,6 +236,9 @@ static int lowpan_header(struct sk_buff *skb, struct net_device *ldev,
> >
> >       memcpy(&info, lowpan_skb_priv(skb), sizeof(info));
> >
> > +     if (info.daddr.mode == IEEE802154_ADDR_LONG)
> > +             lowpan_neigh_short_addr(skb, ldev, &info.daddr);
>
> [Severity: Medium]
> This issue was not introduced by this patch, but is info.daddr.mode ever
> initialized here for AF_PACKET SOCK_RAW senders?
>
> packet_snd() and tpacket_fill_skb() only call dev_hard_header() for
> SOCK_DGRAM:
>
> net/packet/af_packet.c:packet_snd() {
>     ...
>         if (sock->type == SOCK_DGRAM) {
>                 offset = dev_hard_header(skb, dev, ntohs(proto), addr, NULL, len);
>     ...
> }
>
> So for SOCK_RAW, lowpan_header_create() never writes the struct
> lowpan_addr_info at lowpan_skb_priv(skb). That headroom is memory from
> sock_alloc_send_pskb() that has not been zeroed.
>
> The memcpy() above copies it into info, and the new check branches on the
> uninitialized daddr.mode. As before this patch, the rest of the
> uninitialized daddr/saddr still reaches lowpan_header_compress(), the
> ackreq decision and wpan_dev_hard_header().
>
> The next patch in the series, "6lowpan: fix warning in
> lowpan_compress_addr_64()", appears to fix this. It removes
> lowpan_skb_priv() and builds daddr/saddr from the pushed pseudo header and
> wpan_dev.
>
> [ ... ]
>
> > @@ -255,10 +264,12 @@ netdev_tx_t lowpan_xmit(struct sk_buff *skb, struct net_device *ldev)
> >
> >       pr_debug("package xmit\n");
> >
> > -     if (skb->protocol != htons(ETH_P_IPV6)) {
> > +     if (skb->protocol != htons(ETH_P_IPV6) ||
> > +         !pskb_may_pull(skb, sizeof(struct ipv6hdr))) {
> >               kfree_skb(skb);
> >               return NET_XMIT_DROP;
> >       }
> > +     skb_reset_network_header(skb);
>
> [Severity: High]
> This resets only the network header. Can skb->transport_header go stale
> for AF_PACKET SOCK_RAW senders as a result?
>
> At this commit ldev->hard_header_len is sizeof(struct ipv6hdr). For
> SOCK_RAW with len >= 80, packet_snd() therefore leaves the network header
> at data + 40:
>
>         } else if (reserve) {
>                 skb_reserve(skb, -reserve);
>                 if (len < reserve + sizeof(struct ipv6hdr) &&
>                     dev->min_header_len != hard_header_len)
>                         skb_reset_network_header(skb);
>         }
>
> packet_parse_headers()->skb_probe_transport_header() then parses bytes
> 40..79 as the IPv6 header. transport_header ends up around data + 80, or
> stays unset (0xffff) if parsing fails. tpacket_fill_skb() on the TX ring
> path also places the network header hard_header_len bytes after skb->data.
>
> After this reset the network header is at data + 0. If byte 6 (nexthdr) is
> IPPROTO_UDP, lowpan_header_compress()->lowpan_nhc_do_compression() only
> fixes the transport header when the two offsets are equal:
>
> net/6lowpan/nhc.c:lowpan_nhc_do_compression() {
>     ...
>         if (skb->transport_header == skb->network_header)
>                 skb_set_transport_header(skb, sizeof(struct ipv6hdr));
>     ...
> }
>
> That check no longer holds, so udp_compress() does:
>
>         const struct udphdr *uh = udp_hdr(skb);
>
> at the stale offset. That is bytes 80..87, or skb->head + 0xffff if the
> transport header was never set.
>
> Can this read out of bounds, and can the bytes it reads go into the
> transmitted 802.15.4 frame as the UDP ports and checksum?
>
> Only this intermediate commit seems to be affected. The next patch in the
> series, "6lowpan: fix warning in lowpan_compress_addr_64()", sets
> hard_header_len to LOWPAN_PSEUDO_HDR_LEN. It also calls __skb_pull() on the
> pseudo header before skb_reset_network_header(). The reset network header
> then matches where AF_PACKET put it.
>
> Could the two changes be reordered or combined so that this commit is safe
> to bisect?
>
> A separate part of this was not introduced by this patch and is still
> present at the end of the series. pskb_may_pull() only guarantees the IPv6
> header in the linear area (8 + 40 bytes after the next patch). If nexthdr
> is UDP and the payload is shorter than 8 bytes, can udp_compress() read up
> to 8 bytes past skb->tail?
>
> >
> >       WARN_ON_ONCE(skb->len > IPV6_MIN_MTU);
> >
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260928183753.3550210-1-edumazet%40kernel.org