[PATCH net v2] rtase: Workaround for IP fragmented UDP packet hardware bug

[PATCH net v2] rtase: Workaround for IP fragmented UDP packet hardware bug

[Justin Lai]

The hardware parser incorrectly interprets 319/320 in a short
IP fragmented UDP packet payload as standard PTP destination
ports and treats the fragment as a PTP packet for further
parsing.

If the transport data is smaller than RTASE_MIN_PAD_LEN, the
remaining data is insufficient for further parsing and causes
hardware TX hang.

Pad these packets so the transport data reaches
RTASE_MIN_PAD_LEN before transmitting to avoid triggering the
hardware issue.

Fixes: d6e882b89fdf ("rtase: Implement .ndo_start_xmit function")
Cc: stable@vger.kernel.org
Signed-off-by: Justin Lai <justinlai0215@realtek.com>
---
v1 -> v2:
- Remove RTASE_SHORT_PKT_THRESH and the packet length check.
- Check transport data length before parsing the UDP header.
- Add Fixes tag.
- Add Cc: stable@vger.kernel.org.
- Target net tree.
---
 drivers/net/ethernet/realtek/rtase/rtase.h    |  2 +
 .../net/ethernet/realtek/rtase/rtase_main.c   | 50 +++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/drivers/net/ethernet/realtek/rtase/rtase.h b/drivers/net/ethernet/realtek/rtase/rtase.h
index b9209eb6ea73..d489d20177ac 100644
--- a/drivers/net/ethernet/realtek/rtase/rtase.h
+++ b/drivers/net/ethernet/realtek/rtase/rtase.h
@@ -359,4 +359,6 @@ struct rtase_private {
 
 #define RTASE_MSS_MASK GENMASK(28, 18)
 
+#define RTASE_MIN_PAD_LEN 47
+
 #endif /* RTASE_H */
diff --git a/drivers/net/ethernet/realtek/rtase/rtase_main.c b/drivers/net/ethernet/realtek/rtase/rtase_main.c
index 55105d34bc79..6c189b3efab0 100644
--- a/drivers/net/ethernet/realtek/rtase/rtase_main.c
+++ b/drivers/net/ethernet/realtek/rtase/rtase_main.c
@@ -61,6 +61,7 @@
 #include <linux/pci.h>
 #include <linux/pm_runtime.h>
 #include <linux/prefetch.h>
+#include <linux/ptp_classify.h>
 #include <linux/rtnetlink.h>
 #include <linux/tcp.h>
 #include <asm/irq.h>
@@ -1249,6 +1250,52 @@ static u32 rtase_tx_csum(struct sk_buff *skb, const struct net_device *dev)
 	return csum_cmd;
 }
 
+static bool rtase_skb_is_udp(struct sk_buff *skb)
+{
+	int no = skb_network_offset(skb);
+	struct ipv6hdr *i6h, _i6h;
+	struct iphdr *ih, _ih;
+
+	switch (vlan_get_protocol(skb)) {
+	case htons(ETH_P_IP):
+		ih = skb_header_pointer(skb, no, sizeof(_ih), &_ih);
+		return ih && ih->protocol == IPPROTO_UDP;
+	case htons(ETH_P_IPV6):
+		i6h = skb_header_pointer(skb, no, sizeof(_i6h), &_i6h);
+		return i6h && i6h->nexthdr == IPPROTO_UDP;
+	default:
+		return false;
+	}
+}
+
+static bool rtase_skb_pad(struct sk_buff *skb)
+{
+	u32 trans_data_len;
+	u16 dest_port;
+	u32 pad_len;
+
+	if (!skb_transport_header_was_set(skb))
+		return true;
+
+	trans_data_len = skb_tail_pointer(skb) - skb_transport_header(skb);
+	if (trans_data_len < offsetof(struct udphdr, len) ||
+	    trans_data_len >= RTASE_MIN_PAD_LEN)
+		return true;
+
+	if (!rtase_skb_is_udp(skb))
+		return true;
+
+	dest_port = ntohs(udp_hdr(skb)->dest);
+
+	if (dest_port == PTP_EV_PORT || dest_port == PTP_GEN_PORT) {
+		pad_len = RTASE_MIN_PAD_LEN - trans_data_len;
+		if (__skb_put_padto(skb, skb->len + pad_len, false))
+			return false;
+	}
+
+	return true;
+}
+
 static int rtase_xmit_frags(struct rtase_ring *ring, struct sk_buff *skb,
 			    u32 opts1, u32 opts2)
 {
@@ -1362,6 +1409,9 @@ static netdev_tx_t rtase_start_xmit(struct sk_buff *skb,
 		opts2 |= rtase_tx_csum(skb, dev);
 	}
 
+	if (!rtase_skb_pad(skb))
+		goto err_dma_0;
+
 	frags = rtase_xmit_frags(ring, skb, opts1, opts2);
 	if (unlikely(frags < 0))
 		goto err_dma_0;
-- 
2.40.1

Re: [PATCH net v2] rtase: Workaround for IP fragmented UDP packet hardware bug

[Simon Horman]

From: 'Simon Horman' <horms@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.
Full review at: https://sashiko.dev
---
rtase: Workaround for IP fragmented UDP packet hardware bug

The hardware parser incorrectly interprets 319/320 in a short
IP fragmented UDP packet payload as standard PTP destination
ports and treats the fragment as a PTP packet for further
parsing. Pad these packets so the transport data reaches
RTASE_MIN_PAD_LEN before transmitting to avoid triggering the
hardware issue.

> diff --git a/drivers/net/ethernet/realtek/rtase/rtase_main.c b/drivers/net/ethernet/realtek/rtase/rtase_main.c
> index ef13109c49cff..b686b84e15fe2 100644
> --- a/drivers/net/ethernet/realtek/rtase/rtase_main.c
> +++ b/drivers/net/ethernet/realtek/rtase/rtase_main.c

[ ... ]

> @@ -1247,6 +1248,52 @@ static u32 rtase_tx_csum(struct sk_buff *skb, const struct net_device *dev)
>  	return csum_cmd;
>  }
>  
> +static bool rtase_skb_is_udp(struct sk_buff *skb)
> +{
> +	int no = skb_network_offset(skb);
> +	struct ipv6hdr *i6h, _i6h;
> +	struct iphdr *ih, _ih;
> +
> +	switch (vlan_get_protocol(skb)) {
> +	case htons(ETH_P_IP):
> +		ih = skb_header_pointer(skb, no, sizeof(_ih), &_ih);
> +		return ih && ih->protocol == IPPROTO_UDP;
> +	case htons(ETH_P_IPV6):
> +		i6h = skb_header_pointer(skb, no, sizeof(_i6h), &_i6h);
> +		return i6h && i6h->nexthdr == IPPROTO_UDP;

[Severity: High]
Does this check fail to traverse IPv6 extension headers?

If an IPv6 packet contains extension headers, such as an IPv6 fragment
(IPPROTO_FRAGMENT), the nexthdr field will contain the extension header's
protocol number instead of UDP.

Doesn't this bypass the workaround for the exact fragmented packets it is
intended to fix?

> +	default:
> +		return false;
> +	}
> +}
> +
> +static bool rtase_skb_pad(struct sk_buff *skb)
> +{
> +	u32 trans_data_len;
> +	u16 dest_port;
> +	u32 pad_len;
> +
> +	if (!skb_transport_header_was_set(skb))
> +		return true;

[Severity: High]
Does this early return skip the workaround for forwarded (routed) IP
fragments?

The networking stack typically does not set the transport header offset for
forwarded packets or non-first IP fragments.

Could an attacker still trigger the hardware TX hang by routing short,
fragmented UDP packets through this interface?

> +
> +	trans_data_len = skb_tail_pointer(skb) - skb_transport_header(skb);

[Severity: Critical]
Does this calculation miss data stored in paged fragments?

By only subtracting the transport header from the tail pointer, it treats the
linear portion as the entire transport payload length.

For non-linear packets, won't this incorrectly evaluate the payload as
smaller than RTASE_MIN_PAD_LEN and trigger the padding path?

> +	if (trans_data_len < offsetof(struct udphdr, len) ||
> +	    trans_data_len >= RTASE_MIN_PAD_LEN)
> +		return true;
> +
> +	if (!rtase_skb_is_udp(skb))
> +		return true;
> +
> +	dest_port = ntohs(udp_hdr(skb)->dest);
> +
> +	if (dest_port == PTP_EV_PORT || dest_port == PTP_GEN_PORT) {
> +		pad_len = RTASE_MIN_PAD_LEN - trans_data_len;
> +		if (__skb_put_padto(skb, skb->len + pad_len, false))

[Severity: Critical]
Will calling __skb_put_padto() with a length greater than skb->len cause a
kernel panic on non-linear packets?

Since __skb_put_padto() unconditionally invokes __skb_put() when the new
length exceeds the current length, and __skb_put() contains
SKB_LINEAR_ASSERT(skb) which triggers a BUG_ON for non-linear packets, won't
this panic the kernel when processing a fragmented UDP packet?

> +			return false;
> +	}
> +
> +	return true;
> +}
> +
>  static int rtase_xmit_frags(struct rtase_ring *ring, struct sk_buff *skb,
>  			    u32 opts1, u32 opts2)
>  {
> @@ -1360,6 +1407,9 @@ static netdev_tx_t rtase_start_xmit(struct sk_buff *skb,
>  		opts2 |= rtase_tx_csum(skb, dev);
>  	}
>  
> +	if (!rtase_skb_pad(skb))
> +		goto err_dma_0;
> +
>  	frags = rtase_xmit_frags(ring, skb, opts1, opts2);

[Severity: High]
This is a pre-existing issue, but doesn't rtase_xmit_frags() crash on highmem
pages?

The driver advertises NETIF_F_HIGHDMA, but rtase_xmit_frags() attempts to map
SKB fragments using skb_frag_address() and dma_map_single():

addr = skb_frag_address(frag);
mapping = dma_map_single(&tp->pdev->dev, addr, len, DMA_TO_DEVICE);

Since skb_frag_address() returns an invalid pointer for HIGHMEM pages,
shouldn't the driver be using skb_frag_dma_map() instead to prevent panics or
memory corruption?

>  	if (unlikely(frags < 0))
>  		goto err_dma_0;

RE: [PATCH net v2] rtase: Workaround for IP fragmented UDP packet hardware bug

[Justin Lai]

Simon Horman <horms@kernel.org> wrote:
> 
> From: 'Simon Horman' <horms@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.
> Full review at: https://sashiko.dev
> ---
> rtase: Workaround for IP fragmented UDP packet hardware bug
> 
> The hardware parser incorrectly interprets 319/320 in a short IP fragmented
> UDP packet payload as standard PTP destination ports and treats the fragment
> as a PTP packet for further parsing. Pad these packets so the transport data
> reaches RTASE_MIN_PAD_LEN before transmitting to avoid triggering the
> hardware issue.
> 
> > diff --git a/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > b/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > index ef13109c49cff..b686b84e15fe2 100644
> > --- a/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > +++ b/drivers/net/ethernet/realtek/rtase/rtase_main.c
> 
> [ ... ]
> 
> > @@ -1247,6 +1248,52 @@ static u32 rtase_tx_csum(struct sk_buff *skb,
> const struct net_device *dev)
> >       return csum_cmd;
> >  }
> >
> > +static bool rtase_skb_is_udp(struct sk_buff *skb) {
> > +     int no = skb_network_offset(skb);
> > +     struct ipv6hdr *i6h, _i6h;
> > +     struct iphdr *ih, _ih;
> > +
> > +     switch (vlan_get_protocol(skb)) {
> > +     case htons(ETH_P_IP):
> > +             ih = skb_header_pointer(skb, no, sizeof(_ih), &_ih);
> > +             return ih && ih->protocol == IPPROTO_UDP;
> > +     case htons(ETH_P_IPV6):
> > +             i6h = skb_header_pointer(skb, no, sizeof(_i6h), &_i6h);
> > +             return i6h && i6h->nexthdr == IPPROTO_UDP;
> 
> [Severity: High]
> Does this check fail to traverse IPv6 extension headers?
> 
> If an IPv6 packet contains extension headers, such as an IPv6 fragment
> (IPPROTO_FRAGMENT), the nexthdr field will contain the extension header's
> protocol number instead of UDP.
> 
> Doesn't this bypass the workaround for the exact fragmented packets it is
> intended to fix?
> 
Thanks for the review.

Unlike IPv4, IPv6 does not require extension header parsing for
this workaround. The hardware only treats IPv6 packets without
extension headers as PTP packets for further parsing.

Therefore, IPv6 packets carrying extension headers, including
fragments, do not enter this parsing path and are not affected
by the hardware issue addressed by this workaround.

Skipping such packets is intentional.
> > +     default:
> > +             return false;
> > +     }
> > +}
> > +
> > +static bool rtase_skb_pad(struct sk_buff *skb) {
> > +     u32 trans_data_len;
> > +     u16 dest_port;
> > +     u32 pad_len;
> > +
> > +     if (!skb_transport_header_was_set(skb))
> > +             return true;
> 
> [Severity: High]
> Does this early return skip the workaround for forwarded (routed) IP
> fragments?
> 
> The networking stack typically does not set the transport header offset for
> forwarded packets or non-first IP fragments.
> 
> Could an attacker still trigger the hardware TX hang by routing short,
> fragmented UDP packets through this interface?
> 
Thanks for pointing this out.

I agree that relying on skb_transport_header_was_set() is not
robust enough.

I will rework this logic to derive the required offset directly
from the packet contents using skb_header_pointer(), rather
than depending on skb_transport_header_was_set().
> > +
> > +     trans_data_len = skb_tail_pointer(skb) -
> > + skb_transport_header(skb);
> 
> [Severity: Critical]
> Does this calculation miss data stored in paged fragments?
> 
> By only subtracting the transport header from the tail pointer, it treats the
> linear portion as the entire transport payload length.
> 
> For non-linear packets, won't this incorrectly evaluate the payload as smaller
> than RTASE_MIN_PAD_LEN and trigger the padding path?
> 
You're right that skb_tail_pointer() only covers the linear area
and can undercount non-linear skb.

skb->len includes both linear and paged fragments.

trans_data_len = skb->len - skb_transport_offset(skb);
> > +     if (trans_data_len < offsetof(struct udphdr, len) ||
> > +         trans_data_len >= RTASE_MIN_PAD_LEN)
> > +             return true;
> > +
> > +     if (!rtase_skb_is_udp(skb))
> > +             return true;
> > +
> > +     dest_port = ntohs(udp_hdr(skb)->dest);
> > +
> > +     if (dest_port == PTP_EV_PORT || dest_port == PTP_GEN_PORT) {
> > +             pad_len = RTASE_MIN_PAD_LEN - trans_data_len;
> > +             if (__skb_put_padto(skb, skb->len + pad_len, false))
> 
> [Severity: Critical]
> Will calling __skb_put_padto() with a length greater than skb->len cause a
> kernel panic on non-linear packets?
> 
> Since __skb_put_padto() unconditionally invokes __skb_put() when the new
> length exceeds the current length, and __skb_put() contains
> SKB_LINEAR_ASSERT(skb) which triggers a BUG_ON for non-linear packets,
> won't this panic the kernel when processing a fragmented UDP packet?
> 
Thanks for pointing this out.

Before calling __skb_put_padto(), I will check whether the skb is
non-linear. If so, I will first linearize it and then apply the
padding. This avoids calling __skb_put_padto() on a non-linear skb
while still ensuring that the workaround is applied to packets which
may trigger the hardware issue.

if (dest_port == PTP_EV_PORT || dest_port == PTP_GEN_PORT) {
    pad_len = RTASE_MIN_PAD_LEN - trans_data_len;

    if (skb_is_nonlinear(skb)) {
        if (skb_linearize(skb))
            return false;
    }

    if (__skb_put_padto(skb, skb->len + pad_len, false))
        return false;
}
> > +                     return false;
> > +     }
> > +
> > +     return true;
> > +}
> > +
> >  static int rtase_xmit_frags(struct rtase_ring *ring, struct sk_buff *skb,
> >                           u32 opts1, u32 opts2)  { @@ -1360,6
> +1407,9
> > @@ static netdev_tx_t rtase_start_xmit(struct sk_buff *skb,
> >               opts2 |= rtase_tx_csum(skb, dev);
> >       }
> >
> > +     if (!rtase_skb_pad(skb))
> > +             goto err_dma_0;
> > +
> >       frags = rtase_xmit_frags(ring, skb, opts1, opts2);
> 
> [Severity: High]
> This is a pre-existing issue, but doesn't rtase_xmit_frags() crash on highmem
> pages?
> 
> The driver advertises NETIF_F_HIGHDMA, but rtase_xmit_frags() attempts to
> map SKB fragments using skb_frag_address() and dma_map_single():
> 
> addr = skb_frag_address(frag);
> mapping = dma_map_single(&tp->pdev->dev, addr, len, DMA_TO_DEVICE);
> 
> Since skb_frag_address() returns an invalid pointer for HIGHMEM pages,
> shouldn't the driver be using skb_frag_dma_map() instead to prevent panics or
> memory corruption?
> 
Thanks for pointing this out.

This appears to be a pre-existing issue and is unrelated to the change
in this patch. I will investigate whether skb_frag_dma_map() should be
used in rtase_xmit_frags() and address it separately if needed.

Thanks,
Justin
> >       if (unlikely(frags < 0))
> >               goto err_dma_0;

Re: [PATCH net v2] rtase: Workaround for IP fragmented UDP packet hardware bug

[David Laight]

On Tue, 9 Jun 2026 08:20:10 +0000
Justin Lai <justinlai0215@realtek.com> wrote:

> Simon Horman <horms@kernel.org> wrote:
> > 
> > From: 'Simon Horman' <horms@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.
> > Full review at: https://sashiko.dev
> > ---
> > rtase: Workaround for IP fragmented UDP packet hardware bug
> > 
> > The hardware parser incorrectly interprets 319/320 in a short IP fragmented
> > UDP packet payload as standard PTP destination ports and treats the fragment
> > as a PTP packet for further parsing. Pad these packets so the transport data
> > reaches RTASE_MIN_PAD_LEN before transmitting to avoid triggering the
> > hardware issue.
> >   
> > > diff --git a/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > > b/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > > index ef13109c49cff..b686b84e15fe2 100644
> > > --- a/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > > +++ b/drivers/net/ethernet/realtek/rtase/rtase_main.c  
> > 
> > [ ... ]
> >   
> > > @@ -1247,6 +1248,52 @@ static u32 rtase_tx_csum(struct sk_buff *skb,  
> > const struct net_device *dev)  
> > >       return csum_cmd;
> > >  }
> > >
> > > +static bool rtase_skb_is_udp(struct sk_buff *skb) {
> > > +     int no = skb_network_offset(skb);
> > > +     struct ipv6hdr *i6h, _i6h;
> > > +     struct iphdr *ih, _ih;
> > > +
> > > +     switch (vlan_get_protocol(skb)) {
> > > +     case htons(ETH_P_IP):
> > > +             ih = skb_header_pointer(skb, no, sizeof(_ih), &_ih);
> > > +             return ih && ih->protocol == IPPROTO_UDP;
> > > +     case htons(ETH_P_IPV6):
> > > +             i6h = skb_header_pointer(skb, no, sizeof(_i6h), &_i6h);
> > > +             return i6h && i6h->nexthdr == IPPROTO_UDP;  
> > 
> > [Severity: High]
> > Does this check fail to traverse IPv6 extension headers?
> > 
> > If an IPv6 packet contains extension headers, such as an IPv6 fragment
> > (IPPROTO_FRAGMENT), the nexthdr field will contain the extension header's
> > protocol number instead of UDP.
> > 
> > Doesn't this bypass the workaround for the exact fragmented packets it is
> > intended to fix?
> >   
> Thanks for the review.
> 
> Unlike IPv4, IPv6 does not require extension header parsing for
> this workaround. The hardware only treats IPv6 packets without
> extension headers as PTP packets for further parsing.
> 
> Therefore, IPv6 packets carrying extension headers, including
> fragments, do not enter this parsing path and are not affected
> by the hardware issue addressed by this workaround.
> 
> Skipping such packets is intentional.

I thought you said that the problem only arose with packets that are
fragmented by IPv[46], in particular short final fragments.
If your hardware checks for extension headers then doesn't that mean
that you never have a problem with IPv6 packets.

-- David

...