From: Lee Jones <lee@kernel.org>
To: lee@kernel.org
Cc: linux-kernel@vger.kernel.org,
Daniel Starke <daniel.starke@siemens.com>,
Fedor Pchelkin <pchelkin@ispras.ru>,
Jiri Slaby <jirislaby@kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-serial@vger.kernel.org,
syzbot+5f47a8cea6a12b77a876@syzkaller.appspotmail.com
Subject: [PATCH 1/1] tty: n_gsm: Avoid sleeping during .write() whilst atomic
Date: Tue, 3 Oct 2023 18:00:20 +0100 [thread overview]
Message-ID: <20231003170020.830242-1-lee@kernel.org> (raw)
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@siemens.com>
Cc: Fedor Pchelkin <pchelkin@ispras.ru>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-serial@vger.kernel.org
Reported-by: syzbot+5f47a8cea6a12b77a876@syzkaller.appspotmail.com
Signed-off-by: Lee Jones <lee@kernel.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
next reply other threads:[~2023-10-03 17:00 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-03 17:00 Lee Jones [this message]
2023-10-03 18:14 ` [PATCH 1/1] tty: n_gsm: Avoid sleeping during .write() whilst atomic Greg Kroah-Hartman
2023-10-03 18:55 ` Lee Jones
2023-10-04 6:04 ` Greg Kroah-Hartman
2023-10-04 9:09 ` Lee Jones
2023-10-04 9:55 ` Greg Kroah-Hartman
2023-10-04 12:19 ` Greg Kroah-Hartman
2023-10-04 12:57 ` Lee Jones
2023-10-04 13:13 ` Greg Kroah-Hartman
2023-10-05 4:59 ` Jiri Slaby
2023-10-05 6:05 ` Greg Kroah-Hartman
2023-10-05 7:26 ` Lee Jones
2023-10-04 5:59 ` Starke, Daniel
2023-10-04 6:05 ` Greg Kroah-Hartman
2023-10-04 8:40 ` Jiri Slaby
2023-10-04 8:58 ` Starke, Daniel
2023-10-04 8:57 ` Lee Jones
2023-10-04 12:21 ` Greg Kroah-Hartman
2023-10-04 12:57 ` Lee Jones
2023-10-04 13:14 ` Greg Kroah-Hartman
2023-10-05 9:03 ` Lee Jones
2023-10-05 9:18 ` Greg Kroah-Hartman
2023-10-05 10:43 ` Lee Jones
2023-10-05 11:33 ` Greg Kroah-Hartman
2023-10-05 11:38 ` Starke, Daniel
2023-10-05 11:46 ` Lee Jones
2023-10-05 11:55 ` Greg Kroah-Hartman
2023-10-05 12:17 ` Lee Jones
2023-10-05 12:37 ` Greg Kroah-Hartman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231003170020.830242-1-lee@kernel.org \
--to=lee@kernel.org \
--cc=daniel.starke@siemens.com \
--cc=gregkh@linuxfoundation.org \
--cc=jirislaby@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=pchelkin@ispras.ru \
--cc=syzbot+5f47a8cea6a12b77a876@syzkaller.appspotmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.