Skip to content

Commit 902e02e

Browse files
pchelkin91gregkh
pchelkin91
authored and
gregkh
committed
tty: n_gsm: avoid call of sleeping functions from atomic context
Syzkaller reports the following problem: BUG: sleeping function called from invalid context at kernel/printk/printk.c:2347 in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 1105, name: syz-executor423 3 locks held by syz-executor423/1105: #0: ffff8881468b9098 (&tty->ldisc_sem){++++}-{0:0}, at: tty_ldisc_ref_wait+0x22/0x90 drivers/tty/tty_ldisc.c:266 #1: ffff8881468b9130 (&tty->atomic_write_lock){+.+.}-{3:3}, at: tty_write_lock drivers/tty/tty_io.c:952 [inline] #1: ffff8881468b9130 (&tty->atomic_write_lock){+.+.}-{3:3}, at: do_tty_write drivers/tty/tty_io.c:975 [inline] #1: ffff8881468b9130 (&tty->atomic_write_lock){+.+.}-{3:3}, at: file_tty_write.constprop.0+0x2a8/0x8e0 drivers/tty/tty_io.c:1118 #2: ffff88801b06c398 (&gsm->tx_lock){....}-{2:2}, at: gsmld_write+0x5e/0x150 drivers/tty/n_gsm.c:2717 irq event stamp: 3482 hardirqs last enabled at (3481): [<ffffffff81d13343>] __get_reqs_available+0x143/0x2f0 fs/aio.c:946 hardirqs last disabled at (3482): [<ffffffff87d39722>] __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:108 [inline] hardirqs last disabled at (3482): [<ffffffff87d39722>] _raw_spin_lock_irqsave+0x52/0x60 kernel/locking/spinlock.c:159 softirqs last enabled at (3408): [<ffffffff87e01002>] asm_call_irq_on_stack+0x12/0x20 softirqs last disabled at (3401): [<ffffffff87e01002>] asm_call_irq_on_stack+0x12/0x20 Preemption disabled at: [<0000000000000000>] 0x0 CPU: 2 PID: 1105 Comm: syz-executor423 Not tainted 5.10.137-syzkaller #0 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x107/0x167 lib/dump_stack.c:118 ___might_sleep.cold+0x1e8/0x22e kernel/sched/core.c:7304 console_lock+0x19/0x80 kernel/printk/printk.c:2347 do_con_write+0x113/0x1de0 drivers/tty/vt/vt.c:2909 con_write+0x22/0xc0 drivers/tty/vt/vt.c:3296 gsmld_write+0xd0/0x150 drivers/tty/n_gsm.c:2720 do_tty_write drivers/tty/tty_io.c:1028 [inline] file_tty_write.constprop.0+0x502/0x8e0 drivers/tty/tty_io.c:1118 call_write_iter include/linux/fs.h:1903 [inline] aio_write+0x355/0x7b0 fs/aio.c:1580 __io_submit_one fs/aio.c:1952 [inline] io_submit_one+0xf45/0x1a90 fs/aio.c:1999 __do_sys_io_submit fs/aio.c:2058 [inline] __se_sys_io_submit fs/aio.c:2028 [inline] __x64_sys_io_submit+0x18c/0x2f0 fs/aio.c:2028 do_syscall_64+0x33/0x40 arch/x86/entry/common.c:46 entry_SYSCALL_64_after_hwframe+0x61/0xc6 The problem happens in the following control flow: gsmld_write(...) spin_lock_irqsave(&gsm->tx_lock, flags) // taken a spinlock on TX data con_write(...) do_con_write(...) console_lock() might_sleep() // -> bug As far as console_lock() might sleep it should not be called with spinlock held. The patch replaces tx_lock spinlock with mutex in order to avoid the problem. Found by Linux Verification Center (linuxtesting.org) with Syzkaller. Fixes: 32dd59f ("tty: n_gsm: fix race condition in gsmld_write()") Cc: stable <stable@kernel.org> Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru> Link: https://lore.kernel.org/r/20220829131640.69254-3-pchelkin@ispras.ru Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent c9ab053 commit 902e02e

File tree

1 file changed

+24
-29
lines changed

1 file changed

+24
-29
lines changed

‎drivers/tty/n_gsm.c

Lines changed: 24 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ struct gsm_mux {
248248
bool constipated; /* Asked by remote to shut up */
249249
bool has_devices; /* Devices were registered */
250250

251-
spinlock_t tx_lock;
251+
struct mutex tx_mutex;
252252
unsigned int tx_bytes; /* TX data outstanding */
253253
#define TX_THRESH_HI 8192
254254
#define TX_THRESH_LO 2048
@@ -680,7 +680,6 @@ static int gsm_send(struct gsm_mux *gsm, int addr, int cr, int control)
680680
struct gsm_msg *msg;
681681
u8 *dp;
682682
int ocr;
683-
unsigned long flags;
684683

685684
msg = gsm_data_alloc(gsm, addr, 0, control);
686685
if (!msg)
@@ -702,10 +701,10 @@ static int gsm_send(struct gsm_mux *gsm, int addr, int cr, int control)
702701

703702
gsm_print_packet("Q->", addr, cr, control, NULL, 0);
704703

705-
spin_lock_irqsave(&gsm->tx_lock, flags);
704+
mutex_lock(&gsm->tx_mutex);
706705
list_add_tail(&msg->list, &gsm->tx_ctrl_list);
707706
gsm->tx_bytes += msg->len;
708-
spin_unlock_irqrestore(&gsm->tx_lock, flags);
707+
mutex_unlock(&gsm->tx_mutex);
709708
gsmld_write_trigger(gsm);
710709

711710
return 0;
@@ -730,15 +729,15 @@ static void gsm_dlci_clear_queues(struct gsm_mux *gsm, struct gsm_dlci *dlci)
730729
spin_unlock_irqrestore(&dlci->lock, flags);
731730

732731
/* Clear data packets in MUX write queue */
733-
spin_lock_irqsave(&gsm->tx_lock, flags);
732+
mutex_lock(&gsm->tx_mutex);
734733
list_for_each_entry_safe(msg, nmsg, &gsm->tx_data_list, list) {
735734
if (msg->addr != addr)
736735
continue;
737736
gsm->tx_bytes -= msg->len;
738737
list_del(&msg->list);
739738
kfree(msg);
740739
}
741-
spin_unlock_irqrestore(&gsm->tx_lock, flags);
740+
mutex_unlock(&gsm->tx_mutex);
742741
}
743742

744743
/**
@@ -1024,10 +1023,9 @@ static void __gsm_data_queue(struct gsm_dlci *dlci, struct gsm_msg *msg)
10241023

10251024
static void gsm_data_queue(struct gsm_dlci *dlci, struct gsm_msg *msg)
10261025
{
1027-
unsigned long flags;
1028-
spin_lock_irqsave(&dlci->gsm->tx_lock, flags);
1026+
mutex_lock(&dlci->gsm->tx_mutex);
10291027
__gsm_data_queue(dlci, msg);
1030-
spin_unlock_irqrestore(&dlci->gsm->tx_lock, flags);
1028+
mutex_unlock(&dlci->gsm->tx_mutex);
10311029
}
10321030

10331031
/**
@@ -1039,7 +1037,7 @@ static void gsm_data_queue(struct gsm_dlci *dlci, struct gsm_msg *msg)
10391037
* is data. Keep to the MRU of the mux. This path handles the usual tty
10401038
* interface which is a byte stream with optional modem data.
10411039
*
1042-
* Caller must hold the tx_lock of the mux.
1040+
* Caller must hold the tx_mutex of the mux.
10431041
*/
10441042

10451043
static int gsm_dlci_data_output(struct gsm_mux *gsm, struct gsm_dlci *dlci)
@@ -1099,7 +1097,7 @@ static int gsm_dlci_data_output(struct gsm_mux *gsm, struct gsm_dlci *dlci)
10991097
* is data. Keep to the MRU of the mux. This path handles framed data
11001098
* queued as skbuffs to the DLCI.
11011099
*
1102-
* Caller must hold the tx_lock of the mux.
1100+
* Caller must hold the tx_mutex of the mux.
11031101
*/
11041102

11051103
static int gsm_dlci_data_output_framed(struct gsm_mux *gsm,
@@ -1115,7 +1113,7 @@ static int gsm_dlci_data_output_framed(struct gsm_mux *gsm,
11151113
if (dlci->adaption == 4)
11161114
overhead = 1;
11171115

1118-
/* dlci->skb is locked by tx_lock */
1116+
/* dlci->skb is locked by tx_mutex */
11191117
if (dlci->skb == NULL) {
11201118
dlci->skb = skb_dequeue_tail(&dlci->skb_list);
11211119
if (dlci->skb == NULL)
@@ -1169,7 +1167,7 @@ static int gsm_dlci_data_output_framed(struct gsm_mux *gsm,
11691167
* Push an empty frame in to the transmit queue to update the modem status
11701168
* bits and to transmit an optional break.
11711169
*
1172-
* Caller must hold the tx_lock of the mux.
1170+
* Caller must hold the tx_mutex of the mux.
11731171
*/
11741172

11751173
static int gsm_dlci_modem_output(struct gsm_mux *gsm, struct gsm_dlci *dlci,
@@ -1283,13 +1281,12 @@ static int gsm_dlci_data_sweep(struct gsm_mux *gsm)
12831281

12841282
static void gsm_dlci_data_kick(struct gsm_dlci *dlci)
12851283
{
1286-
unsigned long flags;
12871284
int sweep;
12881285

12891286
if (dlci->constipated)
12901287
return;
12911288

1292-
spin_lock_irqsave(&dlci->gsm->tx_lock, flags);
1289+
mutex_lock(&dlci->gsm->tx_mutex);
12931290
/* If we have nothing running then we need to fire up */
12941291
sweep = (dlci->gsm->tx_bytes < TX_THRESH_LO);
12951292
if (dlci->gsm->tx_bytes == 0) {
@@ -1300,7 +1297,7 @@ static void gsm_dlci_data_kick(struct gsm_dlci *dlci)
13001297
}
13011298
if (sweep)
13021299
gsm_dlci_data_sweep(dlci->gsm);
1303-
spin_unlock_irqrestore(&dlci->gsm->tx_lock, flags);
1300+
mutex_unlock(&dlci->gsm->tx_mutex);
13041301
}
13051302

13061303
/*
@@ -1994,14 +1991,13 @@ static void gsm_dlci_command(struct gsm_dlci *dlci, const u8 *data, int len)
19941991
static void gsm_kick_timeout(struct work_struct *work)
19951992
{
19961993
struct gsm_mux *gsm = container_of(work, struct gsm_mux, kick_timeout.work);
1997-
unsigned long flags;
19981994
int sent = 0;
19991995

2000-
spin_lock_irqsave(&gsm->tx_lock, flags);
1996+
mutex_lock(&gsm->tx_mutex);
20011997
/* If we have nothing running then we need to fire up */
20021998
if (gsm->tx_bytes < TX_THRESH_LO)
20031999
sent = gsm_dlci_data_sweep(gsm);
2004-
spin_unlock_irqrestore(&gsm->tx_lock, flags);
2000+
mutex_unlock(&gsm->tx_mutex);
20052001

20062002
if (sent && debug & 4)
20072003
pr_info("%s TX queue stalled\n", __func__);
@@ -2531,6 +2527,7 @@ static void gsm_free_mux(struct gsm_mux *gsm)
25312527
break;
25322528
}
25332529
}
2530+
mutex_destroy(&gsm->tx_mutex);
25342531
mutex_destroy(&gsm->mutex);
25352532
kfree(gsm->txframe);
25362533
kfree(gsm->buf);
@@ -2602,6 +2599,7 @@ static struct gsm_mux *gsm_alloc_mux(void)
26022599
}
26032600
spin_lock_init(&gsm->lock);
26042601
mutex_init(&gsm->mutex);
2602+
mutex_init(&gsm->tx_mutex);
26052603
kref_init(&gsm->ref);
26062604
INIT_LIST_HEAD(&gsm->tx_ctrl_list);
26072605
INIT_LIST_HEAD(&gsm->tx_data_list);
@@ -2610,7 +2608,6 @@ static struct gsm_mux *gsm_alloc_mux(void)
26102608
INIT_WORK(&gsm->tx_work, gsmld_write_task);
26112609
init_waitqueue_head(&gsm->event);
26122610
spin_lock_init(&gsm->control_lock);
2613-
spin_lock_init(&gsm->tx_lock);
26142611

26152612
gsm->t1 = T1;
26162613
gsm->t2 = T2;
@@ -2635,6 +2632,7 @@ static struct gsm_mux *gsm_alloc_mux(void)
26352632
}
26362633
spin_unlock(&gsm_mux_lock);
26372634
if (i == MAX_MUX) {
2635+
mutex_destroy(&gsm->tx_mutex);
26382636
mutex_destroy(&gsm->mutex);
26392637
kfree(gsm->txframe);
26402638
kfree(gsm->buf);
@@ -2790,17 +2788,16 @@ static void gsmld_write_trigger(struct gsm_mux *gsm)
27902788
static void gsmld_write_task(struct work_struct *work)
27912789
{
27922790
struct gsm_mux *gsm = container_of(work, struct gsm_mux, tx_work);
2793-
unsigned long flags;
27942791
int i, ret;
27952792

27962793
/* All outstanding control channel and control messages and one data
27972794
* frame is sent.
27982795
*/
27992796
ret = -ENODEV;
2800-
spin_lock_irqsave(&gsm->tx_lock, flags);
2797+
mutex_lock(&gsm->tx_mutex);
28012798
if (gsm->tty)
28022799
ret = gsm_data_kick(gsm);
2803-
spin_unlock_irqrestore(&gsm->tx_lock, flags);
2800+
mutex_unlock(&gsm->tx_mutex);
28042801

28052802
if (ret >= 0)
28062803
for (i = 0; i < NUM_DLCI; i++)
@@ -3008,21 +3005,20 @@ static ssize_t gsmld_write(struct tty_struct *tty, struct file *file,
30083005
const unsigned char *buf, size_t nr)
30093006
{
30103007
struct gsm_mux *gsm = tty->disc_data;
3011-
unsigned long flags;
30123008
int space;
30133009
int ret;
30143010

30153011
if (!gsm)
30163012
return -ENODEV;
30173013

30183014
ret = -ENOBUFS;
3019-
spin_lock_irqsave(&gsm->tx_lock, flags);
3015+
mutex_lock(&gsm->tx_mutex);
30203016
space = tty_write_room(tty);
30213017
if (space >= nr)
30223018
ret = tty->ops->write(tty, buf, nr);
30233019
else
30243020
set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
3025-
spin_unlock_irqrestore(&gsm->tx_lock, flags);
3021+
mutex_unlock(&gsm->tx_mutex);
30263022

30273023
return ret;
30283024
}
@@ -3319,14 +3315,13 @@ static struct tty_ldisc_ops tty_ldisc_packet = {
33193315
static void gsm_modem_upd_via_data(struct gsm_dlci *dlci, u8 brk)
33203316
{
33213317
struct gsm_mux *gsm = dlci->gsm;
3322-
unsigned long flags;
33233318

33243319
if (dlci->state != DLCI_OPEN || dlci->adaption != 2)
33253320
return;
33263321

3327-
spin_lock_irqsave(&gsm->tx_lock, flags);
3322+
mutex_lock(&gsm->tx_mutex);
33283323
gsm_dlci_modem_output(gsm, dlci, brk);
3329-
spin_unlock_irqrestore(&gsm->tx_lock, flags);
3324+
mutex_unlock(&gsm->tx_mutex);
33303325
}
33313326

33323327
/**

0 commit comments

Comments
 (0)