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]
Date:   Thu, 9 Jan 2020 00:19:19 +0100
From:   RENARD Pierre-Francois <pfrenard@...il.com>
To:     Eric Dumazet <edumazet@...gle.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

OK

Before scp command ( and after a fresh reboot)
---------------------------------
skbuff_ext_cache     378    378    192   21    1 : tunables 0    0    0 
: slabdata     18     18      0
skbuff_fclone_cache    112    112    512   16    2 : tunables 0    0    
0 : slabdata      7      7      0
skbuff_head_cache   1936   2160    256   16    1 : tunables 0    0    0 
: slabdata    135    135      0
---------------------------------
---------------------------------

After the hang of scp (hanged at 203 MB)
---------------------------------
skbuff_ext_cache     693    693    192   21    1 : tunables 0    0    0 
: slabdata     33     33      0
skbuff_fclone_cache    128    128    512   16    2 : tunables 0    0    
0 : slabdata      8      8      0
skbuff_head_cache   2032   2176    256   16    1 : tunables 0    0    0 
: slabdata    136    136      0
---------------------------------
TcpExtTCPSpuriousRtxHostQueues 120                0.0
---------------------------------

After CTRL-C of scp
---------------------------------
skbuff_ext_cache     693    693    192   21    1 : tunables 0    0    0 
: slabdata     33     33      0
skbuff_fclone_cache    128    128    512   16    2 : tunables 0    0    
0 : slabdata      8      8      0
skbuff_head_cache   2112   2336    256   16    1 : tunables 0    0    0 
: slabdata    146    146      0
---------------------------------
TcpExtTCPSpuriousRtxHostQueues  124                0.0
---------------------------------



After the hang of a second attempt (hanged at 1214 MB)
---------------------------------
skbuff_ext_cache     735    735    192   21    1 : tunables 0    0    0 
: slabdata     35     35      0
skbuff_fclone_cache    160    160    512   16    2 : tunables 0    0    
0 : slabdata     10     10      0
skbuff_head_cache   2096   2240    256   16    1 : tunables 0    0    0 
: slabdata    140    140      0
---------------------------------
TcpExtTCPSpuriousRtxHostQueues  248                0.0
---------------------------------


After a third attempt (hanged at 55 MB)
---------------------------------
skbuff_ext_cache     735    735    192   21    1 : tunables 0    0    0 
: slabdata     35     35      0
skbuff_fclone_cache    176    176    512   16    2 : tunables 0    0    
0 : slabdata     11     11      0
skbuff_head_cache   2000   2144    256   16    1 : tunables 0    0    0 
: slabdata    134    134      0
---------------------------------
TcpExtTCPSpuriousRtxHostQueues  365                0.0
---------------------------------








On 1/8/20 9:25 PM, Eric Dumazet wrote:
> 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_;
>>>
>>