netdev - Re: [PATCH net,v3] net: ppp: Fix deadlock caused by unsafe-irq lock in ap

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: Mon, 22 Apr 2024 09:29:37 +0200
From: Jiri Slaby <jirislaby@...nel.org>
To: Jeongjun Park <aha310510@...il.com>
Cc: davem@...emloft.net, edumazet@...gle.com, gregkh@...uxfoundation.org,
 kuba@...nel.org, linux-kernel@...r.kernel.org, linux-ppp@...r.kernel.org,
 netdev@...r.kernel.org, pabeni@...hat.com,
 syzbot+393d0ef63475d9bb1f16@...kaller.appspotmail.com,
 syzkaller-bugs@...glegroups.com
Subject: Re: [PATCH net,v3] net: ppp: Fix deadlock caused by unsafe-irq lock
 in ap_get()

On 22. 04. 24, 9:18, Jeongjun Park wrote:
> ========================================================
> WARNING: possible irq lock inversion dependency detected
> 6.8.0-rc3-syzkaller-00041-g547ab8fc4cb0 #0 Not tainted
> --------------------------------------------------------
...
>    ... key      at: [<ffffffff947ae860>] port_lock_key+0x0/0x20
>    ... acquired at:
>     lock_acquire+0x1e3/0x530 kernel/locking/lockdep.c:5754
>     __raw_read_lock include/linux/rwlock_api_smp.h:150 [inline]
>     _raw_read_lock+0x36/0x50 kernel/locking/spinlock.c:228
>     ap_get drivers/net/ppp/ppp_async.c:136 [inline]
>     ppp_asynctty_wakeup+0x20/0x210 drivers/net/ppp/ppp_async.c:352
>     tty_wakeup+0xbb/0x100 drivers/tty/tty_io.c:523
>     tty_port_default_wakeup+0xa6/0xf0 drivers/tty/tty_port.c:69
>     serial8250_tx_chars+0x6ad/0x8a0 drivers/tty/serial/8250/8250_port.c:1835

> This is the stack trace for the issue causing the deadlock reported
> by syzbot. I think there is definitely a problem with ap_get() and
> this needs to be fixed.

OK, but read_unlock_irq() is not the right way to do it. It would enable 
irqs unconditionally, so serial8250_tx_chars() would not thank you in 
the above call chain.

Plus the issues Jakub had with the patch.

And what about ppp_synctty?

thanks,
-- 
js
suse labs