netdev - Re: [PATCH net] net: usb: lan78xx: fix possible skb leak

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]

Message-ID: <CANn89iKD6DSnz-QdBMYgm=1N2V7UZpCD3TiB+yTO_tGu7XKReg@mail.gmail.com>
Date:   Wed, 8 Jan 2020 12:25:40 -0800
From:   Eric Dumazet <edumazet@...gle.com>
To:     RENARD Pierre-Francois <pfrenard@...il.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>,
        Eric Dumazet <eric.dumazet@...il.com>,
        Stefan Wahren <stefan.wahren@...e.com>,
        Woojung Huh <woojung.huh@...rochip.com>,
        Microchip Linux Driver Support <UNGLinuxDriver@...rochip.com>
Subject: Re: [PATCH net] net: usb: lan78xx: fix possible skb leak

On Wed, Jan 8, 2020 at 11:13 AM RENARD Pierre-Francois
<pfrenard@...il.com> wrote:
>
> I tried with last rawhide kernel 5.5.0-0.rc5.git0.1.local.fc32.aarch64
> I compiled it this night. (I check it includes the patch for lan78xx.c )
>
> Both tests (scp and nfs ) are failing the same way as before.
>
> Fox
>

Please report the output of " grep skb /proc/slabinfo"

before and after your test.

The symptoms (of retransmit being not attempted by TCP) match the fact
that skb(s) is(are) not freed by a driver (or some layer)

When TCP detects this (function skb_still_in_host_queue()), one SNMP
counter is incremented

nstat -a | grep TCPSpuriousRtxHostQueues

Thanks.

>
>
> On 1/7/20 7:57 PM, Eric Dumazet wrote:
> > If skb_linearize() fails, we need to free the skb.
> >
> > TSO makes skb bigger, and this bug might be the reason
> > Raspberry Pi 3B+ users had to disable TSO.
> >
> > Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
> > Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> > Reported-by: RENARD Pierre-Francois <pfrenard@...il.com>
> > Cc: Stefan Wahren <stefan.wahren@...e.com>
> > Cc: Woojung Huh <woojung.huh@...rochip.com>
> > Cc: Microchip Linux Driver Support <UNGLinuxDriver@...rochip.com>
> > ---
> >   drivers/net/usb/lan78xx.c | 9 +++------
> >   1 file changed, 3 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> > index f940dc6485e56a7e8f905082ce920f5dd83232b0..fb4781080d6dec2af22f41c5e064350ea74130b3 100644
> > --- a/drivers/net/usb/lan78xx.c
> > +++ b/drivers/net/usb/lan78xx.c
> > @@ -2724,11 +2724,6 @@ static int lan78xx_stop(struct net_device *net)
> >       return 0;
> >   }
> >
> > -static int lan78xx_linearize(struct sk_buff *skb)
> > -{
> > -     return skb_linearize(skb);
> > -}
> > -
> >   static struct sk_buff *lan78xx_tx_prep(struct lan78xx_net *dev,
> >                                      struct sk_buff *skb, gfp_t flags)
> >   {
> > @@ -2740,8 +2735,10 @@ static struct sk_buff *lan78xx_tx_prep(struct lan78xx_net *dev,
> >               return NULL;
> >       }
> >
> > -     if (lan78xx_linearize(skb) < 0)
> > +     if (skb_linearize(skb)) {
> > +             dev_kfree_skb_any(skb);
> >               return NULL;
> > +     }
> >
> >       tx_cmd_a = (u32)(skb->len & TX_CMD_A_LEN_MASK_) | TX_CMD_A_FCS_;
> >
>
>