Here is a BUG report from syzkaller:
BUG: sleeping function called from invalid context at kernel/printk/printk.c:2565
3 locks held by mingetty/6405:
#0: ffff8881109b7098 (&tty->ldisc_sem){++++}-{0:0},
at: tty_ldisc_ref_wait+0x22/0x80
#1: ffff8881109b7130 (&tty->atomic_write_lock){+.+.}-{3:3},
at: file_tty_write.constprop.0+0x26f/0x8c0
#2: ffff8880147293e0 (&gsm->tx_lock){....}-{2:2},
at: gsmld_write+0x5e/0x140
Call Trace:
__might_resched.cold+0x222/0x26b
console_lock+0x17/0x80
do_con_write+0x10f/0x1e30
con_write+0x21/0x40
gsmld_write+0xcb/0x140
file_tty_write.constprop.0+0x471/0x8c0
vfs_write+0x9ef/0xde0
ksys_write+0x127/0x250
do_syscall_64+0x35/0x80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
And another bug report caused by the same reason is shown as below:
BUG: spinlock wrong CPU on CPU#2, mingetty/30460
lock: 0xffff8880340553c8, .magic: dead4ead, .owner: mingetty/30460, .owner_cpu: 1
Call Trace:
dump_stack_lvl+0xcd/0x134
do_raw_spin_unlock+0x1af/0x230
_raw_spin_unlock_irqrestore+0x1e/0x70
gsmld_write+0xde/0x140
In gsmld_write(), in case of race condition, it would fetch the spin-lock
and disable IRQ. But in the following trace, do_con_write() attempt to down
semaphore which would probably cause re-schedule task and in further, IRQs
woud be ignored for a quite time.
Add if condition in do_con_write(). When the current task is in atomic
context, return immediately.
Fixes: 32dd59f96924 ("tty: n_gsm: fix race condition in gsmld_write()")
Signed-off-by: Zeng Heng <[email protected]>
---
drivers/tty/vt/vt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 981d2bfcf9a5..7662b6eb0836 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -2902,7 +2902,7 @@ static int do_con_write(struct tty_struct *tty, const unsigned char *buf, int co
struct vt_notifier_param param;
bool rescan;
- if (in_interrupt())
+ if (in_interrupt() || irqs_disabled())
return count;
console_lock();
@@ -3358,7 +3358,7 @@ static void con_flush_chars(struct tty_struct *tty)
{
struct vc_data *vc;
- if (in_interrupt()) /* from flush_to_ldisc */
+ if (in_interrupt() || irqs_disabled()) /* from flush_to_ldisc */
return;
/* if we race with con_close(), vt may be null */
--
2.25.1
On 12. 12. 22, 3:35, Zeng Heng wrote:
> Here is a BUG report from syzkaller:
>
> BUG: sleeping function called from invalid context at kernel/printk/printk.c:2565
>
> 3 locks held by mingetty/6405:
> #0: ffff8881109b7098 (&tty->ldisc_sem){++++}-{0:0},
> at: tty_ldisc_ref_wait+0x22/0x80
> #1: ffff8881109b7130 (&tty->atomic_write_lock){+.+.}-{3:3},
> at: file_tty_write.constprop.0+0x26f/0x8c0
> #2: ffff8880147293e0 (&gsm->tx_lock){....}-{2:2},
> at: gsmld_write+0x5e/0x140
>
> Call Trace:
> __might_resched.cold+0x222/0x26b
> console_lock+0x17/0x80
> do_con_write+0x10f/0x1e30
> con_write+0x21/0x40
> gsmld_write+0xcb/0x140
> file_tty_write.constprop.0+0x471/0x8c0
> vfs_write+0x9ef/0xde0
> ksys_write+0x127/0x250
> do_syscall_64+0x35/0x80
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
>
> And another bug report caused by the same reason is shown as below:
>
> BUG: spinlock wrong CPU on CPU#2, mingetty/30460
> lock: 0xffff8880340553c8, .magic: dead4ead, .owner: mingetty/30460, .owner_cpu: 1
> Call Trace:
> dump_stack_lvl+0xcd/0x134
> do_raw_spin_unlock+0x1af/0x230
> _raw_spin_unlock_irqrestore+0x1e/0x70
> gsmld_write+0xde/0x140
>
> In gsmld_write(), in case of race condition, it would fetch the spin-lock
> and disable IRQ. But in the following trace, do_con_write() attempt to down
> semaphore which would probably cause re-schedule task and in further, IRQs
> woud be ignored for a quite time.
Wasn't this fixed in gsmld yet? In any case:
NACK
as it is wrong. This should be fixed in gsmld instead.
> Add if condition in do_con_write(). When the current task is in atomic
> context, return immediately.
>
> Fixes: 32dd59f96924 ("tty: n_gsm: fix race condition in gsmld_write()")
> Signed-off-by: Zeng Heng <[email protected]>
> ---
> drivers/tty/vt/vt.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 981d2bfcf9a5..7662b6eb0836 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -2902,7 +2902,7 @@ static int do_con_write(struct tty_struct *tty, const unsigned char *buf, int co
> struct vt_notifier_param param;
> bool rescan;
>
> - if (in_interrupt())
> + if (in_interrupt() || irqs_disabled())
> return count;
>
> console_lock();
> @@ -3358,7 +3358,7 @@ static void con_flush_chars(struct tty_struct *tty)
> {
> struct vc_data *vc;
>
> - if (in_interrupt()) /* from flush_to_ldisc */
> + if (in_interrupt() || irqs_disabled()) /* from flush_to_ldisc */
> return;
>
> /* if we race with con_close(), vt may be null */
--
js
suse labs