| 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: <57d6fc48-3665-3df1-1c55-48d1b84ef889@gmail.com>
Date: Fri, 10 Jan 2020 11:07:31 -0800
From: Eric Dumazet <eric.dumazet@...il.com>
To: RENARD Pierre-Francois <pfrenard@...il.com>,
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
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