netdev - Re: WARN_ON() in netconsole with PREEMPT

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: <20181130180443.o2kjcpweaglmsmx6@linutronix.de>
Date:   Fri, 30 Nov 2018 19:04:43 +0100
From:   Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To:     Steven Rostedt <rostedt@...dmis.org>,
        Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
Cc:     Oleksandr Natalenko <oleksandr@...alenko.name>,
        "David S. Miller" <davem@...emloft.net>,
        Thomas Gleixner <tglx@...utronix.de>,
        Eric Dumazet <edumazet@...gle.com>,
        Dave Jones <davej@...emonkey.org.uk>, netdev@...r.kernel.org,
        linux-rt-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: WARN_ON() in netconsole with PREEMPT_RT

On 2018-11-11 21:01:05 [-0500], Steven Rostedt wrote:
> On Sun, 11 Nov 2018 21:16:00 +0100
> Oleksandr Natalenko <oleksandr@...alenko.name> wrote:
> > Oh, I see that write_msg() calls netpoll_send_udp() under 
> > spin_lock_irqsave(), but in PREEMPT_RT this, AFAIK, does not disable 
> > interrupts.
> > 
> > So, the real question here is whether the interrupts should be indeed 
> > disabled. And if so, -rt should replace spin_lock_irqsave() call there 
> > with what? local_irq_save()? and get rid of the warning?
> > 
> 
> Why do we care if interrupts are disabled with PREEMPT_RT here?

We don't actually. It was broken before for some reason and disabled on
RT. Later people asked for NETCONSOLE on RT and it seemed to have worked
so I allowed it again starting with v3.14.3-rt5.
A year later (v4.3-rc1) commit c9fd56b34efd0 ("netpoll: warn on
netpoll_send_udp users who haven't disabled irqs") introduced that
warning. 
It seems the only reason why netpoll_send_udp() is invoked with disabled
interrupts is because the caller acquired `target_list_lock' with
disabled interrupts. I remember that earlier there was some
local_irq_save() based locking but all this is gone now.

Nikolay could you please explain why interrupts have to be disabled at
this point? It looks to me that this check could be dropped.

> -- Steve

Sebastian