netdev - Re: [PATCH net] r8152: Fix broken RX checksums.

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] [day] [month] [year] [list]

Message-ID: <d261ef1c-b6d1-18a9-572a-2cf6978ff259@pobox.com>
Date:   Wed, 9 Nov 2016 08:19:31 -0500
From:   Mark Lord <mlord@...ox.com>
To:     Hayes Wang <hayeswang@...ltek.com>,
        David Miller <davem@...emloft.net>
Cc:     nic_swsd <nic_swsd@...ltek.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net] r8152: Fix broken RX checksums.

On 16-11-09 08:09 AM, Hayes Wang wrote:
> Mark Lord [mailto:mlord@...ox.com]
..
>> The MTU/MRU on this link is the standard 1500 bytes, so a pkt_len of 2045 isn't
>> valid here.
>> And the rx_desc values look an awful lot like the rx_data values that follow it.
>>
>> There's definitely more broken here than just TCP RX checksums.
> 
> I don't think it is the issue of our hw. If it happens, windows or
> other OS may have problems, too. It is like the memory issue described
> in commit 990c9b347245("Merge branch 'r8152-fixes'"). It seems that
> the data in memory is not same with the one from the device.

I am still doing long-term testing of various tweaks to the driver,
and can now confirm that changing from kmalloc() to usb_alloc_coherent()
vastly improves reliability, and re-enabling RX checksums works fine
with that change.

However, even with coherent URB buffers, I still see the occasional bad rx_desc:
like, twice in 36 hours of continuous bashing at it.

So having code in the driver to sanitize the rx_desc is essential.
My current test code (shared with Hayes already) includes validation of various
key fields of the rx_desc, and detects when the chip/driver/whatever gets confused.

Hopefully r8152.c will get updated to take more care before trusting
what it sees in the rx_desc fields.

Cheers
-- 
Mark Lord
mlord@...ox.com