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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 27 Jan 2021 15:35:03 -0500
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Heiner Kallweit <hkallweit1@...il.com>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        David Miller <davem@...emloft.net>,
        Realtek linux nic maintainers <nic_swsd@...ltek.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        xplo.bn@...il.com
Subject: Re: [PATCH net] r8169: work around RTL8125 UDP hw bug

On Wed, Jan 27, 2021 at 3:32 PM Heiner Kallweit <hkallweit1@...il.com> wrote:
>
> On 27.01.2021 20:54, Willem de Bruijn wrote:
> > On Wed, Jan 27, 2021 at 2:40 PM Heiner Kallweit <hkallweit1@...il.com> wrote:
> >>
> >> On 27.01.2021 19:07, Willem de Bruijn wrote:
> >>> On Tue, Jan 26, 2021 at 2:40 PM Heiner Kallweit <hkallweit1@...il.com> wrote:
> >>>>
> >>>> 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 <xplo.bn@...il.com>
> >>>> Signed-off-by: Heiner Kallweit <hkallweit1@...il.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.