All of lore.kernel.org
 help / color / mirror / Atom feed
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


             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.