Re: [PATCH net-next v8 2/2] selftests: net: test PPPoE packets in gro.sh

From: 'Simon Horman' <[email protected]>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
selftests: net: test PPPoE packets in gro.sh
    
This commit adds PPPoE test-cases to the GRO selftest and a new "pppoe_sid"
test case to verify that packets with different PPPoE session IDs are correctly
identified as separate flows.

> diff --git a/tools/testing/selftests/net/lib/gro.c b/tools/testing/selftests/net/lib/gro.c
> index 11b16ae5f0e83..4ffb0491c0da7 100644
> --- a/tools/testing/selftests/net/lib/gro.c
> +++ b/tools/testing/selftests/net/lib/gro.c

[ ... ]

> @@ -1071,6 +1095,20 @@ static void send_fragment6(int fd, struct sockaddr_ll *daddr)
>  	write_packet(fd, buf, bufpkt_len, daddr);
>  }
>  
> +static void send_changed_pppoe_sid(int fd, struct sockaddr_ll *daddr)
> +{
> +	static char buf[L2_HLEN_MAX + PAYLOAD_LEN];
> +	int pkt_size = total_hdr_len + PAYLOAD_LEN;
> +	struct pppoe_hdr *hdr = (struct pppoe_hdr *)(buf + ETH_HLEN);
> +
> +	create_packet(buf, 0, 0, PAYLOAD_LEN, 0);

Will this write past the end of the buffer?

When create_packet() is called, it writes total_hdr_len + PAYLOAD_LEN bytes:

tools/testing/selftests/net/lib/gro.c:create_packet() {
    ...
    memset(buf, 0, total_hdr_len);
    memset(buf + total_hdr_len, 'a', payload_len);
    ...
}

However, buf is only sized for L2_HLEN_MAX + PAYLOAD_LEN. L2_HLEN_MAX
evaluates to 22 bytes (ETH_HLEN + PPPOE_SES_HLEN). For IPv6 PPPoE packets,
total_hdr_len can be up to 82 bytes (14 ETH + 8 PPPoE + 40 IPv6 + 20 TCP).

Does this result in an out-of-bounds write of 60 bytes when setting up the
IPv6 test case?

> +	write_packet(fd, buf, pkt_size, daddr);
> +
> +	create_packet(buf, PAYLOAD_LEN, 0, PAYLOAD_LEN, 0);
> +	hdr->sid = htons(0x4321);
> +	write_packet(fd, buf, pkt_size, daddr);
> +}