Re: [Security, resend] Instant crash with rtl8169 and large packets

[Eric Dumazet <dada1@cosmosbay.com>]

Michael Tokarev a écrit :
> Eric Dumazet wrote:
> []
>> OK I suspect driver is buggy since 2.6.10 days :)
>>
>> Could you try this patch ?
> 
> Tried it, and it appears to work.  Tried various MTU combinations
> and packet sizes.  Checked iperf too, with and without the patch
> and with different MTU too, to be sure the patch does not introduce
> any slowdowns - everything looks sane.  In case the incoming packet
> is larger than the RX buffer size, `errors' and `frames' RX stats
> gets incremented.
> 
> The only somewhat odd thing is that rx path accepts packets larger
> than MTU by 3 bytes.  For example, if I set mtu to 2000, the
> largest packet I can send is 2003 bytes; with mtu=2002, largest
> actual packet size is 2005 bytes.  This is complete frame - in
> terms of ping size (ping -s) it's 1975 and 1977 bytes.  That to
> say, maybe we still have some corner case somewhere, for packets
> larger than mtu by 1, 2 or 3 bytes.
> 
> Also I didn't try MTU < 1500.
> 
> Other than that,
> 
> Tested-By: Michael Tokarev <mjt@tls.msk.ru>

Could you confirm this last patch was ok without former two patches ?

> 
> And by the way, your email client uses quoted-printable encoding.
> I had to use trivial perl one-liner to convert your patches to
> plaintext.  JFYI.

Ah yes, this is when I reply to one of your mail, thank you for the hint.

When submitting a new mail, my thunderbird agent uses a regular "Content-Transfer-Encoding: 7bit"


BTW, this driver uses NAPI, but still calls dev_kfree_skb_irq() in rtl8169_tx_interrupt()

You probably can get better performance calling dev_kfree_skb(tx_skb->skb); instead

@@ -3372,7 +3372,7 @@ static void rtl8169_tx_interrupt(struct net_device *dev,
                rtl8169_unmap_tx_skb(tp->pci_dev, tx_skb, tp->TxDescArray + entry);

                if (status & LastFrag) {
-                       dev_kfree_skb_irq(tx_skb->skb);
+                       dev_kfree_skb(tx_skb->skb);
                        tx_skb->skb = NULL;
                }
                dirty_tx++;