| 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
| ||
|
Date: Tue, 3 Oct 2023 18:00:20 +0100
From: Lee Jones <lee@...nel.org>
To: lee@...nel.org
Cc: linux-kernel@...r.kernel.org,
Daniel Starke <daniel.starke@...mens.com>,
Fedor Pchelkin <pchelkin@...ras.ru>,
Jiri Slaby <jirislaby@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
linux-serial@...r.kernel.org,
syzbot+5f47a8cea6a12b77a876@...kaller.appspotmail.com
Subject: [PATCH 1/1] tty: n_gsm: Avoid sleeping during .write() whilst atomic
The fundamental issue here is that gsmld_write() uses spin_lock_irqsave()
to ensure mutual exclusion. spin_lock_irqsave() disables IRQs,
essentially placing the thread of execution into atomic mode and
therefore must not sleep at any point. Unfortunately, the call chain
eventually ends up in __might_resched() which complains loudly.
The BUG() splat looks like this:
BUG: sleeping function called from invalid context at kernel/printk/printk.c:2627
in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 13029, name: (agetty)
preempt_count: 1, expected: 0
RCU nest depth: 0, expected: 0
3 locks held by (agetty)/13029:
#0: ffff888112fc70a0 (&tty->ldisc_sem){++++}-{0:0}, at: tty_ldisc_ref_wait+0x21/0x70
#1: ffff888112fc7130 (&tty->atomic_write_lock){+.+.}-{3:3}, at: file_tty_write+0x1e4/0x9d0
#2: ffff8881053c73e0 (&gsm->tx_lock){....}-{2:2}, at: gsmld_write+0x5b/0x120
irq event stamp: 2506
hardirqs last enabled at (2505): [<ffffffff8b007b0e>] syscall_enter_from_user_mode+0x2e/0x1a0
hardirqs last disabled at (2506): [<ffffffff8b0ab5cc>] _raw_spin_lock_irqsave+0xac/0x120
softirqs last enabled at (514): [<ffffffff81561c9c>] __irq_exit_rcu+0xec/0x160
softirqs last disabled at (509): [<ffffffff81561c9c>] __irq_exit_rcu+0xec/0x160
Preemption disabled at:
[<0000000000000000>] 0x0
CPU: 1 PID: 13029 Comm: (agetty) Not tainted 6.6.0-rc3-next-20230929-00002-gcdf323998d5b #17
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
Call Trace:
<TASK>
dump_stack_lvl+0x1e3/0x2d0
? nf_tcp_handle_invalid+0x650/0x650
? panic+0x8a0/0x8a0
__might_resched+0x5ce/0x790
? __might_sleep+0xc0/0xc0
? reacquire_held_locks+0x680/0x680
console_lock+0x1c/0x1b0
do_con_write+0x118/0x7410
? stack_trace_snprint+0xf0/0xf0
? lockdep_unlock+0x163/0x300
? lockdep_unlock+0x163/0x300
? mark_lock+0x2a0/0x350
? __lock_acquire+0x1302/0x2050
? vt_resize+0xc0/0xc0
? do_raw_spin_lock+0x147/0x3a0
? __rwlock_init+0x140/0x140
? _raw_spin_lock_irqsave+0xdd/0x120
? _raw_spin_lock+0x40/0x40
con_write+0x29/0x640
? check_heap_object+0x249/0x870
gsmld_write+0xf9/0x120
? gsmld_read+0x10/0x10
file_tty_write+0x57c/0x9d0
vfs_write+0x77d/0xaf0
? file_end_write+0x250/0x250
? tty_ioctl+0x8fa/0xbc0
? __fdget_pos+0x1d2/0x330
ksys_write+0x19b/0x2c0
? print_irqtrace_events+0x220/0x220
? __ia32_sys_read+0x80/0x80
? syscall_enter_from_user_mode+0x2e/0x1a0
? syscall_enter_from_user_mode+0x2e/0x1a0
do_syscall_64+0x2b/0x70
entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7f76546101b0
The important part of the call stack being:
gsmld_write() # Takes a lock and disables IRQs
con_write()
console_lock()
__might_sleep()
__might_resched() # Complains that IRQs are disabled
To fix this, let's ensure mutual exclusion by using a protected shared
variable (busy) instead. We'll use the current locking mechanism to
protect it, but ensure that the locks are released and IRQs re-enabled
by the time we transit further down the call chain which may sleep.
Cc: Daniel Starke <daniel.starke@...mens.com>
Cc: Fedor Pchelkin <pchelkin@...ras.ru>
Cc: Jiri Slaby <jirislaby@...nel.org>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: linux-serial@...r.kernel.org
Reported-by: syzbot+5f47a8cea6a12b77a876@...kaller.appspotmail.com
Signed-off-by: Lee Jones <lee@...nel.org>
---
drivers/tty/n_gsm.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 1f3aba607cd51..b83a97d58381f 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -270,6 +270,7 @@ struct gsm_mux {
struct tty_struct *tty; /* The tty our ldisc is bound to */
spinlock_t lock;
struct mutex mutex;
+ bool busy;
unsigned int num;
struct kref ref;
@@ -3253,6 +3254,7 @@ static struct gsm_mux *gsm_alloc_mux(void)
gsm->dead = true; /* Avoid early tty opens */
gsm->wait_config = false; /* Disabled */
gsm->keep_alive = 0; /* Disabled */
+ gsm->busy = false;
/* Store the instance to the mux array or abort if no space is
* available.
@@ -3718,11 +3720,21 @@ static ssize_t gsmld_write(struct tty_struct *tty, struct file *file,
ret = -ENOBUFS;
spin_lock_irqsave(&gsm->tx_lock, flags);
+ if (gsm->busy) {
+ spin_unlock_irqrestore(&gsm->tx_lock, flags);
+ return -EBUSY;
+ }
+ gsm->busy = true;
+ spin_unlock_irqrestore(&gsm->tx_lock, flags);
+
space = tty_write_room(tty);
if (space >= nr)
ret = tty->ops->write(tty, buf, nr);
else
set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
+
+ spin_lock_irqsave(&gsm->tx_lock, flags);
+ gsm->busy = false;
spin_unlock_irqrestore(&gsm->tx_lock, flags);
return ret;
--
2.42.0.582.g8ccd20d70d-goog
Powered by blists - more mailing lists