netdev - Re: netconsole deadlock with virtnet

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

Message-ID: <20201118091257.2ee6757a@gandalf.local.home>
Date:   Wed, 18 Nov 2020 09:12:57 -0500
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Jason Wang <jasowang@...hat.com>
Cc:     Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Leon Romanovsky <leon@...nel.org>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Petr Mladek <pmladek@...e.com>,
        John Ogness <john.ogness@...utronix.de>,
        virtualization@...ts.linux-foundation.org,
        Amit Shah <amit@...nel.org>, Itay Aveksis <itayav@...dia.com>,
        Ran Rozenstein <ranro@...dia.com>,
        netdev <netdev@...r.kernel.org>
Subject: Re: netconsole deadlock with virtnet

[ Adding netdev as perhaps someone there knows ]

On Wed, 18 Nov 2020 12:09:59 +0800
Jason Wang <jasowang@...hat.com> wrote:

> > This CPU0 lock(_xmit_ETHER#2) -> hard IRQ -> lock(console_owner) is
> > basically
> > 	soft IRQ -> lock(_xmit_ETHER#2) -> hard IRQ -> printk()
> >
> > Then CPU1 spins on xmit, which is owned by CPU0, CPU0 spins on
> > console_owner, which is owned by CPU1?  

It still looks to me that the target_list_lock is taken in IRQ, (which can
be the case because printk calls write_msg() which takes that lock). And
someplace there's a:

	lock(target_list_lock)
	lock(xmit_lock)

which means you can remove the console lock from this scenario completely,
and you still have a possible deadlock between target_list_lock and
xmit_lock.

> 
> 
> If this is true, it looks not a virtio-net specific issue but somewhere 
> else.
> 
> I think all network driver will synchronize through bh instead of hardirq.

I think the issue is where target_list_lock is held when we take xmit_lock.
Is there anywhere in netconsole.c that can end up taking xmit_lock while
holding the target_list_lock? If so, that's the problem. As
target_list_lock is something that can be taken in IRQ context, which means
*any* other lock that is taking while holding the target_list_lock must
also protect against interrupts from happening while it they are held.

-- Steve