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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).