Re: [PATCH net] r8169: work around RTL8125 UDP hw bug

On 27.01.2021 21:35, Willem de Bruijn wrote:

On Wed, Jan 27, 2021 at 3:32 PM Heiner Kallweit [off-list ref] wrote:

quoted

On 27.01.2021 20:54, Willem de Bruijn wrote:

quoted

On Wed, Jan 27, 2021 at 2:40 PM Heiner Kallweit [off-list ref] wrote:

quoted

On 27.01.2021 19:07, Willem de Bruijn wrote:

quoted

On Tue, Jan 26, 2021 at 2:40 PM Heiner Kallweit [off-list ref] wrote:

quoted

It was reported that on RTL8125 network breaks under heavy UDP load,
e.g. torrent traffic ([0], from comment 27). Realtek confirmed a hw bug
and provided me with a test version of the r8125 driver including a
workaround. Tests confirmed that the workaround fixes the issue.
I modified the original version of the workaround to meet mainline
code style.

[0] https://bugzilla.kernel.org/show_bug.cgi?id=209839

Fixes: f1bce4ad2f1c ("r8169: add support for RTL8125")
Tested-by: xplo <redacted>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 64 ++++++++++++++++++++---
 1 file changed, 58 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index fb67d8f79..90052033b 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c

@@ -28,6 +28,7 @@
 #include <linux/bitfield.h>
 #include <linux/prefetch.h>
 #include <linux/ipv6.h>
+#include <linux/ptp_classify.h>
 #include <asm/unaligned.h>
 #include <net/ip6_checksum.h>

@@ -4007,17 +4008,64 @@ static int rtl8169_xmit_frags(struct rtl8169_private *tp, struct sk_buff *skb,
        return -EIO;
 }

-static bool rtl_test_hw_pad_bug(struct rtl8169_private *tp)
+static bool rtl_skb_is_udp(struct sk_buff *skb)
 {
+       switch (vlan_get_protocol(skb)) {
+       case htons(ETH_P_IP):
+               return ip_hdr(skb)->protocol == IPPROTO_UDP;
+       case htons(ETH_P_IPV6):
+               return ipv6_hdr(skb)->nexthdr == IPPROTO_UDP;

The workaround was provided by Realtek, I just modified it to match
mainline code style. For your reference I add the original version below.
I don't know where the magic numbers come from, Realtek releases
neither data sheets nor errata information.

Okay. I don't know what is customary for this process.

But I would address the possible out of bounds read by trusting ip
header integrity in rtl_skb_is_udp.

I don't know tun/virtio et al good enough to judge which header elements
may be trustworthy and which may be not. What should be checked where?

It requires treating the transmit path similar to the receive path:
assume malicious or otherwise faulty packets. So do not trust that a
protocol of ETH_P_IPV6 implies a packet with 40B of space to hold a
full ipv6 header. That is the extent of it, really.

OK, so what can I do? Check for
skb_tail_pointer(skb) - skb_network_header(skb) >= sizeof(struct ipv6hdr) ?

On a side note: Why is IP6_HLEN defined in ptp_classify.h and not in any
IPv6 header file? Does no IPv6 code need such a constant?