* [PATCH net v3] rtase: Workaround for TX hang caused by short UDP packets entering hardware PTP parsing
@ 2026-06-15 13:16 Justin Lai
2026-06-17 8:54 ` Simon Horman
2026-06-17 8:59 ` Simon Horman
0 siblings, 2 replies; 5+ messages in thread
From: Justin Lai @ 2026-06-15 13:16 UTC (permalink / raw)
To: kuba
Cc: davem, edumazet, pabeni, andrew+netdev, linux-kernel, netdev,
stable, horms, richardcochran, david.laight.linux,
aleksander.lobakin, pkshih, larry.chiu, Justin Lai
The hardware performs additional PTP parsing on UDP packets identified
by destination ports 319/320 at the expected UDP destination port
offset.
If such a packet has transport data smaller than RTASE_MIN_PAD_LEN,
the parser may access data beyond the end of the packet and trigger a
TX hang.
For IPv4 fragmented packets, a non-initial fragment does not contain a
UDP header. However, if the payload contains values matching PTP
destination ports (319/320) at the expected UDP destination port
offset, the hardware incorrectly classifies the fragment as a PTP
packet and performs further parsing.
IPv6 fragmented packets are not affected because the hardware only
enters this parsing path when the IPv6 Next Header field directly
indicates UDP. Packets carrying a Fragment Header do not enter this
path.
Pad affected packets so the transport data reaches
RTASE_MIN_PAD_LEN before transmission 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>
---
v2 -> v3:
- Remove dependency on skb_transport_header_was_set().
- Determine UDP header offset from IPv4/IPv6 headers.
- Use skb_header_pointer() for UDP header access.
- Add non-linear skb handling.
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 | 79 +++++++++++++++++++
2 files changed, 81 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..4c295a39c7a0 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,81 @@ static u32 rtase_tx_csum(struct sk_buff *skb, const struct net_device *dev)
return csum_cmd;
}
+static bool rtase_get_udp_offset(struct sk_buff *skb, u32 *udp_offset)
+{
+ 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);
+ if (!ih)
+ return false;
+
+ if (ih->ihl < 5)
+ return false;
+
+ if (ih->protocol != IPPROTO_UDP)
+ return false;
+
+ *udp_offset = no + ih->ihl * 4;
+
+ return true;
+ case htons(ETH_P_IPV6):
+ i6h = skb_header_pointer(skb, no, sizeof(_i6h), &_i6h);
+ if (!i6h)
+ return false;
+
+ if (i6h->nexthdr != IPPROTO_UDP)
+ return false;
+
+ *udp_offset = no + sizeof(*i6h);
+
+ return true;
+ default:
+ return false;
+ }
+}
+
+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;
+
+ trans_data_len = skb->len - udp_offset;
+ if (trans_data_len < offsetof(struct udphdr, len) ||
+ trans_data_len >= RTASE_MIN_PAD_LEN)
+ return true;
+
+ dest = skb_header_pointer(skb,
+ udp_offset + offsetof(struct udphdr, dest),
+ sizeof(_dest), &_dest);
+ if (!dest)
+ return true;
+
+ dest_port = ntohs(*dest);
+ if (dest_port != PTP_EV_PORT && dest_port != PTP_GEN_PORT)
+ return true;
+
+ if (skb_is_nonlinear(skb)) {
+ if (skb_linearize(skb))
+ return false;
+ }
+
+ 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 +1438,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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net v3] rtase: Workaround for TX hang caused by short UDP packets entering hardware PTP parsing
2026-06-15 13:16 [PATCH net v3] rtase: Workaround for TX hang caused by short UDP packets entering hardware PTP parsing Justin Lai
@ 2026-06-17 8:54 ` Simon Horman
2026-06-22 10:46 ` Justin Lai
2026-06-17 8:59 ` Simon Horman
1 sibling, 1 reply; 5+ messages in thread
From: Simon Horman @ 2026-06-17 8:54 UTC (permalink / raw)
To: justinlai0215
Cc: Simon Horman, kuba, davem, edumazet, pabeni, andrew+netdev,
linux-kernel, netdev, stable, richardcochran, david.laight.linux,
aleksander.lobakin, pkshih, larry.chiu
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://netdev-ai.bots.linux.dev/sashiko/
---
rtase: Workaround for TX hang caused by short UDP packets entering hardware PTP parsing
This patch adds a workaround for a hardware TX hang that occurs when
short UDP packets matching destination ports 319/320 enter a PTP parsing
path that reads beyond the packet end. The workaround pads affected
packets so the transport data reaches RTASE_MIN_PAD_LEN before
transmission.
> diff --git a/drivers/net/ethernet/realtek/rtase/rtase_main.c b/drivers/net/ethernet/realtek/rtase/rtase_main.c
> index 55105d34bc79..4c295a39c7a0 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)
> +{
> + 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);
> + if (!ih)
> + return false;
> +
> + if (ih->ihl < 5)
> + return false;
> +
> + if (ih->protocol != IPPROTO_UDP)
> + return false;
> +
> + *udp_offset = no + ih->ihl * 4;
[Low]
Should ih->ihl also be checked against the actual data present in the
skb? With ihl up to 15 (60 bytes), if the skb only contains the basic
20-byte IP header, the computed udp_offset can exceed skb->len.
Downstream in rtase_skb_pad():
trans_data_len = skb->len - udp_offset;
trans_data_len is u32, so when udp_offset > skb->len this underflows to
a very large value, which trivially satisfies:
trans_data_len >= RTASE_MIN_PAD_LEN
and rtase_skb_pad() silently skips the padding. A malformed packet from
an AF_PACKET sender with CAP_NET_RAW could craft a mismatched ihl and
bypass the workaround. Would clamping or validating udp_offset against
skb->len before the subtraction be appropriate?
> +
> + return true;
> + case htons(ETH_P_IPV6):
> + i6h = skb_header_pointer(skb, no, sizeof(_i6h), &_i6h);
> + if (!i6h)
> + return false;
> +
> + if (i6h->nexthdr != IPPROTO_UDP)
> + return false;
> +
> + *udp_offset = no + sizeof(*i6h);
> +
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +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;
> +
> + trans_data_len = skb->len - udp_offset;
> + if (trans_data_len < offsetof(struct udphdr, len) ||
> + trans_data_len >= RTASE_MIN_PAD_LEN)
> + return true;
[Low]
When trans_data_len is less than offsetof(struct udphdr, len) (i.e., the
transport region is under 4 bytes), this returns true and skips the
padding entirely.
The commit message states the hardware "may access data beyond the end
of the packet" during PTP parsing. If the hardware also reads out of
bounds while classifying the packet, an IPv4 last fragment with 1-3
trailing bytes could still be misclassified as PTP based on whatever
garbage memory the hardware happens to read, and the workaround would
be bypassed.
The implicit assumption here seems to be "if the driver cannot read the
dest port from the skb, the hardware cannot classify it either", which
appears to contradict the premise that the hardware reads beyond the
packet boundary. Should these very short transport-data cases also be
padded to be safe?
> +
> + dest = skb_header_pointer(skb,
> + udp_offset + offsetof(struct udphdr, dest),
> + sizeof(_dest), &_dest);
[ ... ]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net v3] rtase: Workaround for TX hang caused by short UDP packets entering hardware PTP parsing
2026-06-15 13:16 [PATCH net v3] rtase: Workaround for TX hang caused by short UDP packets entering hardware PTP parsing Justin Lai
2026-06-17 8:54 ` Simon Horman
@ 2026-06-17 8:59 ` Simon Horman
2026-06-22 10:56 ` Justin Lai
1 sibling, 1 reply; 5+ messages in thread
From: Simon Horman @ 2026-06-17 8:59 UTC (permalink / raw)
To: justinlai0215
Cc: Simon Horman, kuba, davem, edumazet, pabeni, andrew+netdev,
linux-kernel, netdev, stable, richardcochran, david.laight.linux,
aleksander.lobakin, pkshih, larry.chiu
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 TX hang caused by short UDP packets entering hardware PTP parsing
The patch adds a workaround for a hardware bug in rtase where short UDP
packets triggering PTP parsing can cause a TX hang due to out-of-bounds reads.
It pads affected packets so the transport data reaches a minimum length.
Cc: stable@vger.kernel.org
> diff --git a/drivers/net/ethernet/realtek/rtase/rtase_main.c b/drivers/net/ethernet/realtek/rtase/rtase_main.c
> index 55105d34bc797..4c295a39c7a0c 100644
> --- a/drivers/net/ethernet/realtek/rtase/rtase_main.c
> +++ b/drivers/net/ethernet/realtek/rtase/rtase_main.c
[ ... ]
> @@ -1249,6 +1250,81 @@ static u32 rtase_tx_csum(struct sk_buff *skb, const struct net_device *dev)
> return csum_cmd;
> }
>
> +static bool rtase_get_udp_offset(struct sk_buff *skb, u32 *udp_offset)
> +{
> + int no = skb_network_offset(skb);
> + struct ipv6hdr *i6h, _i6h;
> + struct iphdr *ih, _ih;
> +
> + switch (vlan_get_protocol(skb)) {
> + case htons(ETH_P_IP):
[Severity: High]
Does relying on vlan_get_protocol(skb) allow an unprivileged user to bypass
this workaround?
If an attacker uses an AF_PACKET/SOCK_RAW socket to inject a raw Ethernet
frame containing an IPv4/UDP packet, they could set skb->protocol to an
arbitrary value like htons(ETH_P_ALL). The switch statement will fall
through to the default case and return false, causing rtase_skb_pad() to
skip the padding.
Since the hardware independently parses the raw frame based on the EtherType,
could it still process the unpadded packet and trigger the TX hang?
> + ih = skb_header_pointer(skb, no, sizeof(_ih), &_ih);
> + if (!ih)
> + return false;
[ ... ]
> +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;
> +
> + 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]
Does this logic inadvertently skip padding for underflowed lengths and tiny
payloads?
If the IP header is malformed such that skb->len < udp_offset, the unsigned
subtraction for trans_data_len will underflow to a large positive value.
This causes trans_data_len >= RTASE_MIN_PAD_LEN to be true, bypassing the
padding.
Additionally, if the packet has fewer than 4 bytes of UDP payload, meaning
trans_data_len < offsetof(struct udphdr, len), it explicitly returns true.
Since the hardware blindly fetches the UDP destination port at
udp_offset + 2 via DMA, won't transmitting these short or malformed packets
without padding still expose the hardware to out-of-bounds reads?
> +
> + dest = skb_header_pointer(skb,
> + udp_offset + offsetof(struct udphdr, dest),
> + sizeof(_dest), &_dest);
[ ... ]
> @@ -1362,6 +1438,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;
[Severity: High]
This is a pre-existing issue, but does the err_dma_1 error path in
rtase_start_xmit() contain a double free of the skb?
When transmitting a fragmented packet, rtase_xmit_frags() maps the
fragments and assigns ring->skbuff[entry] = skb for the last fragment
descriptor. Back in rtase_start_xmit(), if dma_map_single() fails for the
linear part, it jumps to err_dma_1:
err_dma_1:
ring->skbuff[entry] = NULL;
rtase_tx_clear_range(ring, ring->cur_idx + 1, frags);
err_dma_0:
tp->stats.tx_dropped++;
dev_kfree_skb_any(skb);
return NETDEV_TX_OK;
rtase_tx_clear_range() iterates over the mapped fragments and calls
dev_kfree_skb_any() when it finds the skb pointer. Execution then falls
through to err_dma_0, which unconditionally calls dev_kfree_skb_any(skb) a
second time on the same skb pointer.
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH net v3] rtase: Workaround for TX hang caused by short UDP packets entering hardware PTP parsing
2026-06-17 8:54 ` Simon Horman
@ 2026-06-22 10:46 ` Justin Lai
0 siblings, 0 replies; 5+ messages in thread
From: Justin Lai @ 2026-06-22 10:46 UTC (permalink / raw)
To: Simon Horman
Cc: kuba@kernel.org, davem@davemloft.net, edumazet@google.com,
pabeni@redhat.com, andrew+netdev@lunn.ch,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
stable@vger.kernel.org, richardcochran@gmail.com,
david.laight.linux@gmail.com, aleksander.lobakin@intel.com,
Ping-Ke Shih, Larry Chiu
Simon Horman <horms@kernel.org> wrote:
>
> 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://netdev-ai.bots.linux.dev/sashiko/
> ---
> rtase: Workaround for TX hang caused by short UDP packets entering
> hardware PTP parsing
>
> This patch adds a workaround for a hardware TX hang that occurs when
> short UDP packets matching destination ports 319/320 enter a PTP parsing
> path that reads beyond the packet end. The workaround pads affected
> packets so the transport data reaches RTASE_MIN_PAD_LEN before
> transmission.
>
> > diff --git a/drivers/net/ethernet/realtek/rtase/rtase_main.c
> b/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > index 55105d34bc79..4c295a39c7a0 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)
> > +{
> > + 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);
> > + if (!ih)
> > + return false;
> > +
> > + if (ih->ihl < 5)
> > + return false;
> > +
> > + if (ih->protocol != IPPROTO_UDP)
> > + return false;
> > +
> > + *udp_offset = no + ih->ihl * 4;
>
> [Low]
> Should ih->ihl also be checked against the actual data present in the
> skb? With ihl up to 15 (60 bytes), if the skb only contains the basic
> 20-byte IP header, the computed udp_offset can exceed skb->len.
>
> Downstream in rtase_skb_pad():
>
> trans_data_len = skb->len - udp_offset;
>
> trans_data_len is u32, so when udp_offset > skb->len this underflows to
> a very large value, which trivially satisfies:
>
> trans_data_len >= RTASE_MIN_PAD_LEN
>
> and rtase_skb_pad() silently skips the padding. A malformed packet from
> an AF_PACKET sender with CAP_NET_RAW could craft a mismatched ihl and
> bypass the workaround. Would clamping or validating udp_offset against
> skb->len before the subtraction be appropriate?
>
Thanks for pointing this out.
I'll add a check for udp_offset > skb->len before calculating
trans_data_len to avoid the potential underflow.
If the computed UDP offset is beyond the packet length, the packet is
malformed and cannot be safely handled by this workaround, so it should
not be transmitted.
> > +
> > + return true;
> > + case htons(ETH_P_IPV6):
> > + i6h = skb_header_pointer(skb, no, sizeof(_i6h), &_i6h);
> > + if (!i6h)
> > + return false;
> > +
> > + if (i6h->nexthdr != IPPROTO_UDP)
> > + return false;
> > +
> > + *udp_offset = no + sizeof(*i6h);
> > +
> > + return true;
> > + default:
> > + return false;
> > + }
> > +}
> > +
> > +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;
> > +
> > + trans_data_len = skb->len - udp_offset;
> > + if (trans_data_len < offsetof(struct udphdr, len) ||
> > + trans_data_len >= RTASE_MIN_PAD_LEN)
> > + return true;
>
> [Low]
> When trans_data_len is less than offsetof(struct udphdr, len) (i.e., the
> transport region is under 4 bytes), this returns true and skips the
> padding entirely.
>
> The commit message states the hardware "may access data beyond the end
> of the packet" during PTP parsing. If the hardware also reads out of
> bounds while classifying the packet, an IPv4 last fragment with 1-3
> trailing bytes could still be misclassified as PTP based on whatever
> garbage memory the hardware happens to read, and the workaround would
> be bypassed.
>
> The implicit assumption here seems to be "if the driver cannot read the
> dest port from the skb, the hardware cannot classify it either", which
> appears to contradict the premise that the hardware reads beyond the
> packet boundary. Should these very short transport-data cases also be
> padded to be safe?
>
Thanks for the review.
The out-of-bounds access occurs after the packet has already matched
the PTP classification criteria (UDP destination port 319/320) and
enters the hardware PTP parsing path.
If the transport data is shorter than the destination port field
itself, the packet cannot match the affected PTP classification path.
Thanks,
Justin
> > +
> > + dest = skb_header_pointer(skb,
> > + udp_offset + offsetof(struct udphdr,
> dest),
> > + sizeof(_dest), &_dest);
>
> [ ... ]
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH net v3] rtase: Workaround for TX hang caused by short UDP packets entering hardware PTP parsing
2026-06-17 8:59 ` Simon Horman
@ 2026-06-22 10:56 ` Justin Lai
0 siblings, 0 replies; 5+ messages in thread
From: Justin Lai @ 2026-06-22 10:56 UTC (permalink / raw)
To: Simon Horman
Cc: kuba@kernel.org, davem@davemloft.net, edumazet@google.com,
pabeni@redhat.com, andrew+netdev@lunn.ch,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
stable@vger.kernel.org, richardcochran@gmail.com,
david.laight.linux@gmail.com, aleksander.lobakin@intel.com,
Ping-Ke Shih, Larry Chiu
Simon Horman <horms@kernel.org> wrote:
>
> 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 TX hang caused by short UDP packets entering
> hardware PTP parsing
>
> The patch adds a workaround for a hardware bug in rtase where short UDP
> packets triggering PTP parsing can cause a TX hang due to out-of-bounds
> reads.
> It pads affected packets so the transport data reaches a minimum length.
>
> Cc: stable@vger.kernel.org
>
> > diff --git a/drivers/net/ethernet/realtek/rtase/rtase_main.c
> b/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > index 55105d34bc797..4c295a39c7a0c 100644
> > --- a/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > +++ b/drivers/net/ethernet/realtek/rtase/rtase_main.c
> [ ... ]
> > @@ -1249,6 +1250,81 @@ static u32 rtase_tx_csum(struct sk_buff *skb,
> const struct net_device *dev)
> > return csum_cmd;
> > }
> >
> > +static bool rtase_get_udp_offset(struct sk_buff *skb, u32 *udp_offset)
> > +{
> > + int no = skb_network_offset(skb);
> > + struct ipv6hdr *i6h, _i6h;
> > + struct iphdr *ih, _ih;
> > +
> > + switch (vlan_get_protocol(skb)) {
> > + case htons(ETH_P_IP):
>
> [Severity: High]
> Does relying on vlan_get_protocol(skb) allow an unprivileged user to bypass
> this workaround?
>
> If an attacker uses an AF_PACKET/SOCK_RAW socket to inject a raw Ethernet
> frame containing an IPv4/UDP packet, they could set skb->protocol to an
> arbitrary value like htons(ETH_P_ALL). The switch statement will fall
> through to the default case and return false, causing rtase_skb_pad() to
> skip the padding.
>
> Since the hardware independently parses the raw frame based on the
> EtherType,
> could it still process the unpadded packet and trigger the TX hang?
>
Thanks for pointing this out.
Since the hardware parser operates on the actual Ethernet frame
contents, relying on skb protocol metadata is not ideal for this
workaround.
I'll update the code to derive the L3 protocol and network offset from
the Ethernet/VLAN headers in the skb data before parsing IPv4/IPv6.
This makes the software check match what the hardware parser sees and
avoids depending on skb protocol metadata.
> > + ih = skb_header_pointer(skb, no, sizeof(_ih), &_ih);
> > + if (!ih)
> > + return false;
> [ ... ]
> > +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;
> > +
> > + 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]
> Does this logic inadvertently skip padding for underflowed lengths and tiny
> payloads?
>
> If the IP header is malformed such that skb->len < udp_offset, the unsigned
> subtraction for trans_data_len will underflow to a large positive value.
> This causes trans_data_len >= RTASE_MIN_PAD_LEN to be true, bypassing the
> padding.
>
> Additionally, if the packet has fewer than 4 bytes of UDP payload, meaning
> trans_data_len < offsetof(struct udphdr, len), it explicitly returns true.
>
> Since the hardware blindly fetches the UDP destination port at
> udp_offset + 2 via DMA, won't transmitting these short or malformed packets
> without padding still expose the hardware to out-of-bounds reads?
>
Thanks for the review.
I'll add a check for udp_offset > skb->len before calculating
trans_data_len to avoid the potential underflow.
If the computed UDP offset is beyond the packet length, the packet is
malformed and cannot be safely handled by this workaround, so it should
not be transmitted.
The out-of-bounds access occurs after the packet has already matched
the PTP classification criteria (UDP destination port 319/320) and
enters the hardware PTP parsing path.
If the transport data is shorter than the destination port field
itself, the packet cannot match the affected PTP classification path.
> > +
> > + dest = skb_header_pointer(skb,
> > + udp_offset + offsetof(struct udphdr,
> dest),
> > + sizeof(_dest), &_dest);
> [ ... ]
> > @@ -1362,6 +1438,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;
>
> [Severity: High]
> This is a pre-existing issue, but does the err_dma_1 error path in
> rtase_start_xmit() contain a double free of the skb?
>
> When transmitting a fragmented packet, rtase_xmit_frags() maps the
> fragments and assigns ring->skbuff[entry] = skb for the last fragment
> descriptor. Back in rtase_start_xmit(), if dma_map_single() fails for the
> linear part, it jumps to err_dma_1:
>
> err_dma_1:
> ring->skbuff[entry] = NULL;
> rtase_tx_clear_range(ring, ring->cur_idx + 1, frags);
>
> err_dma_0:
> tp->stats.tx_dropped++;
> dev_kfree_skb_any(skb);
> return NETDEV_TX_OK;
>
> rtase_tx_clear_range() iterates over the mapped fragments and calls
> dev_kfree_skb_any() when it finds the skb pointer. Execution then falls
> through to err_dma_0, which unconditionally calls dev_kfree_skb_any(skb) a
> second time on the same skb pointer.
Thanks for pointing this out.
This appears to be a pre-existing issue and is unrelated to the change
in this patch.
I'll investigate the reported double free scenario further and address
it separately if a fix is needed.
Thanks,
Justin
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-06-22 11:06 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-15 13:16 [PATCH net v3] rtase: Workaround for TX hang caused by short UDP packets entering hardware PTP parsing Justin Lai
2026-06-17 8:54 ` Simon Horman
2026-06-22 10:46 ` Justin Lai
2026-06-17 8:59 ` Simon Horman
2026-06-22 10:56 ` Justin Lai
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.