| 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
| ||
|
Message-ID: <b9440da5-3682-634f-fca8-7cb70bae8581@gmail.com>
Date: Sat, 11 Jan 2020 09:20:19 +0100
From: RENARD Pierre-Francois <pfrenard@...il.com>
To: Eric Dumazet <eric.dumazet@...il.com>,
Eric Dumazet <edumazet@...gle.com>
Cc: "David S . Miller" <davem@...emloft.net>,
netdev <netdev@...r.kernel.org>,
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
Hello,
I applied patch and I am NOT able to reproduce the issue with both NFS
or SCP !
Congratulations !
Fox
On 1/10/20 8:07 PM, Eric Dumazet wrote:
>
> On 1/8/20 3:19 PM, RENARD Pierre-Francois wrote:
>> 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
>> ---------------------------------
>>
>>
>
> Thanks for testing.
>
> This seems to suggest there is another bug in the driver, leading to some skb being never freed.
>
> Since the driver seems to limit aggregation to 9000 bytes (MAX_SINGLE_PACKET_SIZE)
> I wonder if gso skbs should also be limited to the same value.
>
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index d3239b49c3bb2803c874a2e8af332bcf03848e18..65dea9a94b90e27889c8f44294560ffeabda2eb9 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -3787,6 +3787,8 @@ static int lan78xx_probe(struct usb_interface *intf,
> if (ret < 0)
> goto out4;
>
> + netif_set_gso_max_size(netdev, MAX_SINGLE_PACKET_SIZE - MAX_HEADER);
> +
> ret = register_netdev(netdev);
> if (ret != 0) {
> netif_err(dev, probe, netdev, "couldn't register the device\n");
>
>
>
>>
>>
>>
>>
>>
>> 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_;
>>>>>
Powered by blists - more mailing lists