In `lowpan_header_compress()` (`net/6lowpan/iphc.c`) and
`lowpan_nhc_do_compression()` (`net/6lowpan/nhc.c`), the transmit
compression path reads `ipv6_hdr(skb)` (`sizeof(struct ipv6hdr)`) and
the next-header transport header (`nhc->nexthdrlen`, e.g.,
`sizeof(struct udphdr)`), and then calls `skb_pull(skb,
nhc->nexthdrlen)` and `skb_pull(skb, sizeof(struct ipv6hdr))` without
verifying via `pskb_may_pull()` that those headers are present in the
linear data area of `skb`.

When a short or non-linear `ETH_P_IPV6` frame is transmitted over a
6LoWPAN interface (for example, via `AF_PACKET`),
`lowpan_header_compress()` and `lowpan_nhc_do_compression()` read past
`skb_tail_pointer(skb)` and trigger `BUG_ON(skb->len < skb->data_len)`
in `skb_pull()`.

Check `pskb_may_pull(skb, sizeof(struct ipv6hdr))` in
`lowpan_header_compress()` and `pskb_may_pull(skb, sizeof(struct
ipv6hdr) + nhc->nexthdrlen)` in `lowpan_nhc_do_compression()`.

Fixes: 8df8c56a5abc ("6lowpan: Moving generic compression code into 6lowpan_iphc.c")
Fixes: 92aa7c65d295 ("6lowpan: add generic nhc layer interface")
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>

---
 net/6lowpan/iphc.c | 3 +++
 net/6lowpan/nhc.c  | 5 +++++
 2 files changed, 8 insertions(+)

diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
index 37eaff3f7b69..b2be70036ee7 100644
--- a/net/6lowpan/iphc.c
+++ b/net/6lowpan/iphc.c
@@ -1140,6 +1140,9 @@ int lowpan_header_compress(struct sk_buff *skb, const struct net_device *dev,
 	if (skb->protocol != htons(ETH_P_IPV6))
 		return -EINVAL;
 
+	if (!pskb_may_pull(skb, sizeof(struct ipv6hdr)))
+		return -EINVAL;
+
 	hdr = ipv6_hdr(skb);
 	hc_ptr = head + 2;
 
diff --git a/net/6lowpan/nhc.c b/net/6lowpan/nhc.c
index a4dde85664f2..a9bf340c090a 100644
--- a/net/6lowpan/nhc.c
+++ b/net/6lowpan/nhc.c
@@ -79,6 +79,11 @@ int lowpan_nhc_do_compression(struct sk_buff *skb, const struct ipv6hdr *hdr,
 		goto out;
 	}
 
+	if (!pskb_may_pull(skb, sizeof(struct ipv6hdr) + nhc->nexthdrlen)) {
+		ret = -EINVAL;
+		goto out;
+	}
+
 	/* In the case of RAW sockets the transport header is not set by
 	 * the ip6 stack so we must set it ourselves
 	 */
-- 
2.55.0.1082.g2b9226bbc0-goog

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

Critical: 0 · High: 2 · Medium: 1 · Low: 1

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] Incomplete fix: the same unvalidated IPv6-header dereference on
  the 6LoWPAN transmit path survives in both link-layer callers, which…
- [High] Both new guards are anchored at skb->data, but the reads they
  must protect are anchored at header offsets, so the checks can pass…
- [Medium] The two new pskb_may_pull() failures only produce -EINVAL, and
  neither in-tree caller honours it, so the malformed frame this patch…
- [Low] Commit message states the wrong failure mode, but the concern's
  own mechanism is also disproven.

--

6lowpan: add missing pskb_may_pull() checks in IPHC and NHC compression

This adds a pskb_may_pull() check for sizeof(struct ipv6hdr) in
lowpan_header_compress() and for sizeof(struct ipv6hdr) +
nhc->nexthdrlen in lowpan_nhc_do_compression(), returning -EINVAL when
the headers are not in the linear area.

A few questions on the changelog and on the placement of the checks.

> 6lowpan: add missing pskb_may_pull() checks in IPHC and NHC compression
>
>     When a short or non-linear `ETH_P_IPV6` frame is transmitted over a
>     6LoWPAN interface (for example, via `AF_PACKET`),
>     `lowpan_header_compress()` and `lowpan_nhc_do_compression()` read past
>     `skb_tail_pointer(skb)` and trigger `BUG_ON(skb->len < skb->data_len)`
>     in `skb_pull()`.

[Severity: Low]
Is the described failure mode accurate?  skb_pull() is skb_pull_inline():

include/linux/skbuff.h:
	return unlikely(len > skb->len) ? NULL : __skb_pull(skb, len);

For the reachable case, a short but linear ETH_P_IPV6 frame (skb->len = 6,
data_len = 0), the skb_pull(skb, sizeof(struct ipv6hdr)) in
lowpan_header_compress() just returns NULL without touching skb->len, so
the BUG() inside __skb_pull() cannot be reached.

The non-linear variant also looks unreachable for these two netdevs:
neither lowpan_setup() (net/ieee802154/6lowpan/core.c) nor netdev_setup()
(net/bluetooth/6lowpan.c) advertises NETIF_F_SG or NETIF_F_FRAGLIST, so
skb_needs_linearize() is true and validate_xmit_skb() linearizes or drops
any paged/fraglist skb before ndo_start_xmit.

Would it be more precise to describe this as an out-of-bounds read of the
IPv6 header whose bytes are then emitted in the compressed header, rather
than as a BUG()?

> diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
> index 37eaff3f7b69..b2be70036ee7 100644
> --- a/net/6lowpan/iphc.c
> +++ b/net/6lowpan/iphc.c
> @@ -1140,6 +1140,9 @@ int lowpan_header_compress(struct sk_buff *skb, const struct net_device *dev,
>  	if (skb->protocol != htons(ETH_P_IPV6))
>  		return -EINVAL;
>  
> +	if (!pskb_may_pull(skb, sizeof(struct ipv6hdr)))
> +		return -EINVAL;
> +

[Severity: High]
Does this cover the whole transmit path?  Both link-layer callers
dereference the IPv6 header before lowpan_header_compress() is reached.

net/bluetooth/6lowpan.c:setup_header() {
	...
	hdr = ipv6_hdr(skb);
	...
	memcpy(&ipv6_daddr, &hdr->daddr, sizeof(ipv6_daddr));
	...
	lowpan_header_compress(skb, netdev, daddr, dev->netdev->dev_addr);
}

The memcpy() reads 16 bytes at IPv6 header offsets 24..39 and the result
feeds ipv6_addr_is_multicast(), BT_DBG("dest IP %pI6c") and
peer_lookup_dst(), all before the new check runs.  netdev_setup() sets
hard_header_len = 0 and bt_xmit() only calls skb_unshare(), which
validates no length, so an AF_PACKET send of 6 bytes with sll_protocol =
htons(ETH_P_IPV6) makes those bytes land past skb_tail_pointer(skb).

net/ieee802154/6lowpan/tx.c:lowpan_header_create() {
	const struct ipv6hdr *hdr = ipv6_hdr(skb);
	...
	n = neigh_lookup(&nd_tbl, &hdr->daddr, ldev);
}

This is header_ops->create, called from dev_hard_header() in
packet_snd() on the SOCK_DGRAM path before skb_copy_datagram_from_iter()
has copied any payload, with no pskb_may_pull() anywhere.

Should the length and linearity validation happen at the xmit/hard-header
entry points (bt_xmit()/setup_header() before the daddr read, and
lowpan_header_create()) instead of only inside the compressor?

[Severity: Medium]
Who consumes this new -EINVAL?  Neither in-tree caller looks at it.

net/ieee802154/6lowpan/tx.c:lowpan_header() {
	...
	lowpan_header_compress(skb, ldev, &info.daddr, &info.saddr);

	*dgram_offset = (*dgram_size - skb->len) + skb_network_header_len(skb);
}

lowpan_header() returns only wpan_dev_hard_header()'s value, so the
"if (ret < 0) drop" in lowpan_xmit() is never taken.
net/bluetooth/6lowpan.c:setup_header() also calls it as a void statement
and then reports its own unicast/multicast status, so bt_xmit() goes on to
dev_hard_header()/send_pkt().

Does that mean the frame this patch rejects is still put on the link,
uncompressed?  Its first byte 0x6x aliases LOWPAN_DISPATCH_IPHC (0x60,
mask 0xE0), so a receiver would parse it as an IPHC header.

There is a second effect on the ieee802154 path: the aborted compression
never reaches the skb_pull()/skb_reset_transport_header() at the end of
lowpan_header_compress(), so skb_network_header_len() is called on an skb
whose transport header may still be the ~0U sentinel:

include/linux/skbuff.h:skb_network_header_len() {
	DEBUG_NET_WARN_ON_ONCE(!skb_transport_header_was_set(skb));
	return skb->transport_header - skb->network_header;
}

Can this trip the debug warning and produce a garbage *dgram_offset?

For completeness: the stronger claim that lowpan_xmit_fragment() then
copies past the tail did not hold up — a frame short enough to fail
pskb_may_pull() always satisfies
skb_tail_pointer(skb) - skb_network_header(skb) <= max_single and takes
the single-frame path.

Would it work to have both callers propagate the error and drop the skb,
or to move the check into lowpan_xmit()/bt_xmit()?

>  	hdr = ipv6_hdr(skb);
>  	hc_ptr = head + 2;
>  
> diff --git a/net/6lowpan/nhc.c b/net/6lowpan/nhc.c
> index a4dde85664f2..a9bf340c090a 100644
> --- a/net/6lowpan/nhc.c
> +++ b/net/6lowpan/nhc.c
> @@ -79,6 +79,11 @@ int lowpan_nhc_do_compression(struct sk_buff *skb, const struct ipv6hdr *hdr,
>  		goto out;
>  	}
>  
> +	if (!pskb_may_pull(skb, sizeof(struct ipv6hdr) + nhc->nexthdrlen)) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +

[Severity: High]
Are these guards anchored at the right pointer?  pskb_may_pull() measures
from skb->data:

include/linux/skbuff.h:pskb_may_pull_reason() {
	if (likely(len <= skb_headlen(skb)))
		return SKB_NOT_DROPPED_YET;
}

but the reads they protect are anchored at header offsets:
lowpan_header_compress() uses hdr = ipv6_hdr(skb), i.e.
skb->head + skb->network_header, and the only in-tree nhc->compress,
udp_compress() (net/6lowpan/nhc_udp.c), reads
udp_hdr(skb) == skb->head + skb->transport_header.  When
skb_network_offset(skb) != 0 the checks validate a different range than
the one dereferenced.

A fully linear case on an ieee802154 6LoWPAN device (lowpan_setup() sets
hard_header_len = sizeof(struct ipv6hdr), min_header_len = 0): send exactly
80 bytes over AF_PACKET SOCK_RAW with sll_protocol = htons(ETH_P_IPV6),
bytes 40..79 being an IPv6 header with nexthdr = 17.

In packet_snd() reserve = 40, the skb is fully linear,
skb_reset_network_header() followed by skb_reserve(skb, -40) leaves
skb_network_offset(skb) == 40, and the
"len < reserve + sizeof(struct ipv6hdr)" re-reset is skipped since 80 is
not < 80.  Then:

net/core/flow_dissector.c:__skb_flow_dissect() {
	key_control->thoff = min_t(u16, nhoff, skb ? skb->len : hlen);
}

gives thoff = min(80, 80) = 80, so the transport header points exactly at
skb_tail_pointer(skb).  pskb_may_pull(skb, 40) passes (headlen 80),
nexthdr 17 selects the UDP NHC, pskb_may_pull(skb, 48) also passes, and
udp_compress() reads uh->source, uh->dest and uh->check at skb->data +
80..88.  Does lowpan_push_hc_data() then copy those eight uninitialised
bytes into the header that is transmitted on the link?

The IPHC guard looks affected the same way by any non-zero network offset,
e.g. a tc mirred ingress-to-egress redirect that does
skb_push_rcsum(skb, skb->mac_len) before dev_queue_xmit(), giving
network_offset 14 and a 14-byte past-tail read of hdr for skb->len in
40..53.

Would pskb_may_pull(skb, skb_network_offset(skb) + sizeof(struct ipv6hdr))
and skb_transport_offset(skb) + nhc->nexthdrlen be the right forms here,
or should a non-zero skb_network_offset(skb) simply be rejected?

>  	/* In the case of RAW sockets the transport header is not set by
>  	 * the ip6 stack so we must set it ourselves
>  	 */

On the adjacent pre-existing fixup:

	if (skb->transport_header == skb->network_header)
		skb_set_transport_header(skb, sizeof(struct ipv6hdr));

This isn't a bug introduced by this patch, but the canonical test for an
unset transport header is !skb_transport_header_was_set(skb):

include/linux/skbuff.h:skb_transport_header_was_set() {
	return skb->transport_header != (typeof(skb->transport_header))~0U;
}

With the equality test above, an skb whose transport header was never set
escapes the fixup and udp_hdr(skb) resolves to skb->head + 0xFFFF.  Is it
worth switching to skb_transport_header_was_set() while touching this
function?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260919213638.3314763-1-benquike%40gmail.com