[PATCH net v2] batman-adv: reject unrepresentable multicast TVLV offsets

[PATCH net v2] batman-adv: reject unrepresentable multicast TVLV offsets

[David Lee]

From: Kyle Zeng <kylebot@openai.com>

The network and transport header fields in struct sk_buff are 16-bit
offsets from skb->head, and U16_MAX is reserved as the unset transport
header value. batadv_tvlv_call_handler() sets both fields from a received
multicast TVLV without checking whether the TVLV end is representable.

If the end offset exceeds the field's range, skb_set_transport_header()
truncates it so that the transport header precedes the network header.
The negative difference is then returned by skb_network_header_len() as
a large u32. batadv_mcast_forw_packet() consequently accepts an oversized
multicast tracker and accesses memory beyond the skb data.

Add skb_set_transport_header_careful(), an offset-aware counterpart to
skb_reset_transport_header_careful(), which validates the final
head-relative offset before assigning it. Use the new helper in
batadv_tvlv_call_handler() and reject unrepresentable TVLVs before
setting the network header.

Fixes: 07afe1ba288c ("batman-adv: mcast: implement multicast packet reception and forwarding")
Cc: stable@vger.kernel.org
Suggested-by: Sven Eckelmann <sven@narfation.org>
Assisted-by: Codex:gpt-5.6-sol Codex:gpt-5.5-cyber
Signed-off-by: Kyle Zeng <kylebot@openai.com>
Co-developed-by: David Lee <david.lee@trailofbits.com>
Signed-off-by: David Lee <david.lee@trailofbits.com>
---
Changes in v2:
- Add an offset-aware careful transport-header setter and make batman-adv
  handle its failure, as suggested by Sven Eckelmann.
- Restore Kyle Zeng as the patch author and correct the sign-off chain.
- Move the research credit below the commit-message separator.

v1: https://lore.kernel.org/all/20260731135222.566367-1-david.lee@trailofbits.com/

Bug found and triaged by OpenAI Security Research and
validated by Trail of Bits.

Trail of Bits has a reproducer for this bug that triggers a
KASAN use-after-free and can share if needed.

 include/linux/skbuff.h | 26 ++++++++++++++++++++++++++
 net/batman-adv/tvlv.c  |  5 ++++-
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 22eda1d54a0e..dbeceaf5c3b3 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3126,6 +3126,32 @@ static inline void skb_set_transport_header(struct sk_buff *skb,
 	skb->transport_header += offset;
 }
 
+/**
+ * skb_set_transport_header_careful - conditionally set transport header
+ * @skb: buffer to alter
+ * @offset: offset to add to skb->data
+ *
+ * Hardened version of skb_set_transport_header().
+ *
+ * Returns: true if the operation was a success.
+ */
+static inline bool __must_check
+skb_set_transport_header_careful(struct sk_buff *skb, const int offset)
+{
+	long transport_offset = skb->data - skb->head + offset;
+
+	if (unlikely(transport_offset !=
+		     (typeof(skb->transport_header))transport_offset))
+		return false;
+
+	if (unlikely(transport_offset ==
+		     (typeof(skb->transport_header))~0U))
+		return false;
+
+	skb->transport_header = transport_offset;
+	return true;
+}
+
 static inline unsigned char *skb_network_header(const struct sk_buff *skb)
 {
 	return skb->head + skb->network_header;
diff --git a/net/batman-adv/tvlv.c b/net/batman-adv/tvlv.c
index 1c9fb21985f6..c384db16bcac 100644
--- a/net/batman-adv/tvlv.c
+++ b/net/batman-adv/tvlv.c
@@ -433,8 +433,11 @@ static int batadv_tvlv_call_handler(struct batadv_priv *bat_priv,
 			return NET_RX_SUCCESS;
 
 		tvlv_offset = (unsigned char *)tvlv_value - skb->data;
+		if (!skb_set_transport_header_careful(skb,
+						      tvlv_offset + tvlv_value_len))
+			return -EINVAL;
+
 		skb_set_network_header(skb, tvlv_offset);
-		skb_set_transport_header(skb, tvlv_offset + tvlv_value_len);
 
 		return tvlv_handler->mcast_handler(bat_priv, skb);
 	}
-- 
2.53.0

Re: [PATCH net v2] batman-adv: reject unrepresentable multicast TVLV offsets

[Sven Eckelmann]

> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h                                                                                                                                                                
> index 22eda1d54a0e8..dbeceaf5c3b39 100644                                                                                                                                                                                   
> --- a/include/linux/skbuff.h                                                                                                                                                                                                
> +++ b/include/linux/skbuff.h                                                                                                                                                                                                
> @@ -3126,6 +3126,32 @@ static inline void skb_set_transport_header(struct sk_buff *skb
> [...]
> +static inline bool __must_check
> +skb_set_transport_header_careful(struct sk_buff *skb, const int offset)
> +{
> +	long transport_offset = skb->data - skb->head + offset;
> +
> +	if (unlikely(transport_offset !=
> +		     (typeof(skb->transport_header))transport_offset))
> +		return false;
> +
> +	if (unlikely(transport_offset ==
> +		     (typeof(skb->transport_header))~0U))
> +		return false;
> +
> +	skb->transport_header = transport_offset;
> +	return true;
> +}
> +

I personally don't like the way the lines are broken down. The 80 character
limit was replaced by a 100 character limit since commit bdc48fa11e46
("checkpatch/coding-style: deprecate 80-column warning"). And it seems like
there are already multiple lines in this file which are longer than 80
characters. So it should be possible not to add a line break right after a
comparison operator.

But I leave the judgement about this part to the netdev maintainers.

> diff --git a/net/batman-adv/tvlv.c b/net/batman-adv/tvlv.c
> index de907c07fa154..93492d393c347 100644
> --- a/net/batman-adv/tvlv.c
> +++ b/net/batman-adv/tvlv.c
> @@ -438,8 +438,11 @@ static int batadv_tvlv_call_handler(struct batadv_priv *bat_priv,
>  			return NET_RX_SUCCESS;
>  
>  		tvlv_offset = (unsigned char *)tvlv_value - skb->data;
> +		if (!skb_set_transport_header_careful(skb,
> +						      tvlv_offset + tvlv_value_len))
> +			return -EINVAL;
> +
>  		skb_set_network_header(skb, tvlv_offset);
> -		skb_set_transport_header(skb, tvlv_offset + tvlv_value_len);
>  
>  		return tvlv_handler->mcast_handler(bat_priv, skb);
>  	}

For the batman-adv part:

Acked-by: Sven Eckelmann <sven@narfation.org>

-- 
Sven Eckelmann <sven@narfation.org>