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

On Fri, 31 Jul 2026 13:52:22 +0000, David Lee [off-list ref] wrote:

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 without checking
that the end of a received multicast TVLV is representable.

A sufficiently large TVLV therefore wraps the transport header offset.
skb_network_header_len() then returns a bogus large length, allowing
batadv_mcast_forw_packet() to access memory beyond the skb data.

Ok, we end up here with transport_header < network_header. This is then
wrapping in

static inline u32 skb_network_header_len(const struct sk_buff *skb)
{
	DEBUG_NET_WARN_ON_ONCE(!skb_transport_header_was_set(skb));
	return skb->transport_header - skb->network_header;
}

to a really large value.

And as result, following allows larger tvlv_lens than are actually practical
possible:

	/* check if full tracker tvlv is within skb length */
	tvlv_len = sizeof(*mcast_tracker) + ETH_ALEN * num_dests;
	if (tvlv_len > skb_network_header_len(skb))
		return -EINVAL;

Just because tvlv_len is insane large, correct?


@netdev: Just to sidetrack a little bit: I am wondering if `transport_header <
network_header` should also be handled in skb_network_header_len() (and similar
functions)? And if the return value of u32 should be reduced to u16.

In this specific case, a return value of u16 would have returned the correct
result instead of the insane large length - but trigger and WARN_ON_ONCE for
transport_header == U16_MAX case.

Reject multicast TVLVs whose absolute end offset cannot be represented
before changing either header field.

Fixes: 07afe1ba288c ("batman-adv: mcast: implement multicast packet reception and forwarding")
Bug found and triaged by OpenAI Security Research and
validated by Trail of Bits.

Assisted-by: Codex:gpt-5.6-sol gpt-5.5-cyber

This is not how we write the tags section of a commit message. The "Bug
found..." looks like it was accidentally added and doesn't belong here. And it
seems like this was only added by you for the patches you've send out today:

https://lore.kernel.org/all/?q=Bug%20found%20and%20triaged%20by%20OpenAI%20Security%20Research%20and


See https://docs.kernel.org/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
for more documentation how you can document such things.

How do you want to procede here? Because the rest looks good.

Just for documentation purposes:

This is (skb->data - skb->head) + tvlv_offset + tvlv_value_len

The calculation in skb_set_transport_header():

offset = skb->data - skb->head
offset += (tvlv_offset + tvlv_value_len)

 		skb_set_network_header(skb, tvlv_offset);
 		skb_set_transport_header(skb, tvlv_offset + tvlv_value_len);
 

Just a lazy way to show that the return value u16 returns more sane results:

 #include <stdint.h>
 #include <stdio.h>

// #define RETURN_TEST_TYPE uint32_t
 #define RETURN_TEST_TYPE uint16_t

uint16_t network_header(unsigned long headroom, const int offset)
{
	return headroom + offset;
}

uint16_t transport_header(unsigned long headroom, const int offset)
{
	return headroom + offset;
}

RETURN_TEST_TYPE network_header_len(uint16_t network_header, uint16_t transport_header)
{
	// ignore this for now: DEBUG_NET_WARN_ON_ONCE(!skb_transport_header_was_set(skb));
	return transport_header - network_header;
}

int main(void)
{
        unsigned int tvlv_value_len;
        unsigned int tvlv_offset;
	unsigned int headroom;
	uint16_t transport;
	uint16_t network;
	unsigned int t;

	for (headroom = 0; headroom < 65535; headroom++) {
		for (tvlv_offset = 0; tvlv_offset < 65535; tvlv_offset++) {
			for (tvlv_value_len = 0; tvlv_value_len < 65535; tvlv_value_len++) {
				network = network_header(headroom, tvlv_offset);
				transport = transport_header(headroom,
                                                             tvlv_offset + tvlv_value_len);
				t = network_header_len(network, transport);

				if (t != tvlv_value_len)
					printf("Failed for headroom %u tvlv_offset %u, expected %u, seen %u\n",
					       headroom, tvlv_offset, tvlv_value_len, t);
			}
		}
	}

	return 0;
}

-- 
Sven Eckelmann [off-list ref]