netdev - Re: [PATCH net 2/2] r8169: disable interrupts also for GRO-scheduled NAPI

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]

Date: Tue, 14 May 2024 07:11:00 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Alexander Lobakin <aleksander.lobakin@...el.com>
Cc: Eric Dumazet <edumazet@...gle.com>, Heiner Kallweit
 <hkallweit1@...il.com>, Paolo Abeni <pabeni@...hat.com>, David Miller
 <davem@...emloft.net>, Realtek linux nic maintainers
 <nic_swsd@...ltek.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
 Ken Milmore <ken.milmore@...il.com>
Subject: Re: [PATCH net 2/2] r8169: disable interrupts also for
 GRO-scheduled NAPI

On Tue, 14 May 2024 13:05:55 +0200 Eric Dumazet wrote:
> > napi_schedule()         // we disabled interrupts
> > napi_poll()             // we polled < budget frames
> > napi_complete_done()    // reenable the interrupts, no repoll
> >   hrtimer_start()       // GRO flush is queued
> >     napi_schedule()
> >       napi_poll()       // GRO flush, BUT interrupts are enabled

I thought the bug is because of a race with disable.
But there's already a synchronize_net() after disable, so NAPI poll
must fully exit before we mask in rtl8169_cleanup().

If the bug is double-enable you describe the fix is just making 
the race window smaller. But I don't think that's the bug.

BTW why are events only acked in rtl8169_interrupt() and not
rtl8169_poll()?