Re: [PATCH net v4] rtase: Workaround for TX hang caused by short UDP packets entering hardware PTP parsing

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
    rtase: Workaround for TX hang caused by short UDP packets entering hardware PTP parsing

This commit attempts to work around a hardware TX hang caused by out-of-bounds
reads during PTP parsing. It introduces logic to pad short UDP packets to a
minimum transport data length before transmission.

> diff --git a/drivers/net/ethernet/realtek/rtase/rtase_main.c b/drivers/net/ethernet/realtek/rtase/rtase_main.c
> index 255667775f0e8..944ffe71b2d07 100644
> --- a/drivers/net/ethernet/realtek/rtase/rtase_main.c
> +++ b/drivers/net/ethernet/realtek/rtase/rtase_main.c
[ ... ]
> +static bool rtase_get_udp_offset(struct sk_buff *skb, u32 *udp_offset)
> +{
> +	struct ipv6hdr *i6h, _i6h;
> +	struct iphdr *ih, _ih;
> +	__be16 proto;
> +	u32 no;
> +
> +	if (!rtase_get_l3_proto(skb, &proto, &no))
> +		return false;
> +
> +	switch (proto) {
> +	case htons(ETH_P_IP):
> +		ih = skb_header_pointer(skb, no, sizeof(_ih), &_ih);
> +		if (!ih)
> +			return false;

[Severity: High]
Does this skip the padding workaround if the IP header itself is truncated?

If the packet is shorter than the IP header (e.g., less than 34 bytes),
skb_header_pointer() will return false. This causes rtase_skb_pad() to
return true, allowing the packet to skip padding. Could this small unpadded
packet still trigger the hardware parser and cause the TX hang?

[ ... ]
> +static bool rtase_skb_pad(struct sk_buff *skb)
> +{
> +	__be16 *dest, _dest;
> +	u32 trans_data_len;
> +	u32 udp_offset;
> +	u16 dest_port;
> +	u32 pad_len;
> +
> +	if (!rtase_get_udp_offset(skb, &udp_offset))
> +		return true;
> +
> +	if (udp_offset > skb->len)
> +		return false;
> +
> +	trans_data_len = skb->len - udp_offset;
> +	if (trans_data_len < offsetof(struct udphdr, len) ||
> +	    trans_data_len >= RTASE_MIN_PAD_LEN)
> +		return true;

[Severity: High]
Can an attacker bypass the workaround if the transport payload is very small?

By returning true when trans_data_len is less than
offsetof(struct udphdr, len) (which is 4 bytes), the packet is passed to
the hardware unpadded.

If an attacker sends an IPv4 non-initial fragment with a very small payload,
the hardware parser might identify the fragment as UDP and attempt to read the
destination port at offset 2 from the payload. If this out-of-bounds read
hits residual uninitialized memory in the TX FIFO containing PTP port values
(319 or 320), the hardware could incorrectly classify it as a PTP packet,
continue parsing out of bounds, and trigger the TX hang.

> +
> +	dest = skb_header_pointer(skb,
-- 
pw-bot: cr