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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAK8P3a0u+usoPon7aNOAB_g+Jzkhbz9Q7-vyYci1ReHB6c-JMQ@mail.gmail.com>
Date:   Thu, 24 Jun 2021 10:28:21 +0200
From:   Arnd Bergmann <arnd@...nel.org>
To:     "Maciej W. Rozycki" <macro@...am.me.uk>
Cc:     Nikolai Zhubr <zhubr.2@...il.com>,
        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.

On Thu, Jun 24, 2021 at 1:39 AM Maciej W. Rozycki <macro@...am.me.uk> wrote:
>
> On Wed, 23 Jun 2021, Nikolai Zhubr wrote:
>
> > > As I said before, I still think we should also merge the 8139 driver patch,
> > > probably without that loop. It's not great, but I'm much more confident
> > > I understand what that does and that the patched version is better than
> > > the current code.
> >
> > Yes, the 'poll' approach apparently works stable and does not cause any
> > measurable performance decrease. But it would need some carefull
> > cleanup/review, especially WRT locking.

Right, I forgot you saw that one WARN_ON trigger. If you enable KALLSYMS
and BUGVERBOSE, it should become obvious what that one is about.

> >  Now that all real event handling work
> > is happening in the poll function, it still has to be protected against the
> > (potentially also long-running) reset function which in current design can be
> > called e.g. from a different thread due to tx timeout, and this does not look
> > good, but it is a bit beyond my capability to arrange it better. Besides, the
> > idea was to keep the fix simple and avoid a massive rework...

Since the calls are just moved from hardirq to softirq context, it should
be possible to do this without changing the locking state of any bit, by
just making sure that irqs are disabled during the code that was part of
the hardirq handler.

If you remove the loop, you can move the spin_lock_irqsave(&tp->lock)
back down  after the rx handler.

Anything on top of that can be done as a cleanup or simplification.

>  The simplest fix is not always the right one.

Sure, but in this case, a fairly simple change can help make this driver
work on any machine that for whatever reason treats the IRQ as
edge triggered.

>  I've now posted a small patch series that adds a PCI IRQ router for the
> SiS85C497 device.

Thanks a lot, that should help avoid the same problem on
this chipset with other drivers.

        Arnd