netdev - Re: Realtek 8139 problem on 486.

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <60E75057.60706@gmail.com>
Date:   Thu, 08 Jul 2021 22:21:59 +0300
From:   Nikolai Zhubr <zhubr.2@...il.com>
To:     Arnd Bergmann <arnd@...nel.org>
CC:     "Maciej W. Rozycki" <macro@...am.me.uk>,
        Thomas Gleixner <tglx@...utronix.de>,
        Heiner Kallweit <hkallweit1@...il.com>,
        netdev <netdev@...r.kernel.org>,
        the arch/x86 maintainers <x86@...nel.org>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>
Subject: Re: Realtek 8139 problem on 486.

Hello Arnd,

03.07.2021 12:10, Arnd Bergmann:
> The simplest workaround would be to just move the
> "spin_lock_irqsave(&tp->lock, flags);" a few lines down, below the rx
> processing. This keeps the locking rules exactly how they were before

Indeed, moving spin_lock_irqsave below rtl8139_rx eliminated the warn_on 
message apparently, here is a new log:

https://pastebin.com/dVFNVEu4

and here is my resulting diff:

https://pastebin.com/CzNzsUPu

My usual tests run fine. However I still see 2 issues:

1. I do not understand all this locking thing enough to do a good 
cleanup myself, and it looks like it needs one;
2. It looks like in case of incorrect (edge) triggering mode, the "poll 
approach" with no loop added in the poll function would still allow a 
race window, as explained in following outline (from some previous mails):

22.06.2021 14:12, David Laight:
 > Typically you need to:
 > 1) stop the chip driving IRQ low.
 > 2) process all the completed RX and TX entries.
 > 3) clear the chip's interrupt pending bits (often write to clear).
 > 4) check for completed RX/TX entries, back to 2 if found.
 > 5) enable driving IRQ.
 >
 > The loop (4) is needed because of the timing window between
 > (2) and (3).
 > You can swap (2) and (3) over - but then you get an additional
 > interrupt if packets arrive during processing - which is common.

So in terms of such outline, the "poll approach" now implements 1, 2, 3, 
5 but still misses 4, and my understanding is that it is therefore still 
not a complete solution for the broken triggering case (Although 
practically, the time window might be too small for the race effect to 
be ever observable) From my previous testing I know that such a loop 
does not affect the perfomance too much anyway, so it seems quite safe 
to add it. Maybe I've missunderstood something though.

Thank you,

Regards,
Nikolai

> your patch, and anything beyond that could be done as a follow-up
> cleanup, if someone wants to think this through more.
>
>          Arnd
>