WARNING in __might_sleep (2)

[syzbot]

Hello,

syzbot found the following crash on:

HEAD commit: 8fe28cb58bcb Linux 4.20
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=16211a3b400000
kernel config: https://syzkaller.appspot.com/x/.config?x=7d581260bae0899a
dashboard link: https://syzkaller.appspot.com/bug?extid=c244af085a0159d22879
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1617cd8b400000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=11f5089b400000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+c244af...@syzkaller.appspotmail.com

audit: type=1800 audit(1545700018.193:29): pid=7508 uid=0 auid=4294967295
ses=4294967295 subj==unconfined op=collect_data cause=failed(directio)
comm="startpar" name="rc.local" dev="sda1" ino=2432 res=0
------------[ cut here ]------------
do not call blocking ops when !TASK_RUNNING; state=1 set at
[<000000004df58f46>] n_hdlc_tty_read+0x301/0x7f0 drivers/tty/n_hdlc.c:589
WARNING: CPU: 0 PID: 7659 at kernel/sched/core.c:6099
__might_sleep+0x13d/0x190 kernel/sched/core.c:6094
Kernel panic - not syncing: panic_on_warn set ...
CPU: 0 PID: 7659 Comm: syz-executor111 Not tainted 4.20.0 #387
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1d3/0x2c6 lib/dump_stack.c:113
panic+0x2ad/0x55c kernel/panic.c:188
__warn.cold.8+0x20/0x45 kernel/panic.c:540
report_bug+0x254/0x2d0 lib/bug.c:186
fixup_bug arch/x86/kernel/traps.c:178 [inline]
do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:271
do_invalid_op+0x36/0x40 arch/x86/kernel/traps.c:290
invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:973
RIP: 0010:__might_sleep+0x13d/0x190 kernel/sched/core.c:6094
Code: 65 48 8b 1c 25 40 ee 01 00 48 8d 7b 10 48 89 fe 48 c1 ee 03 80 3c 06
00 75 2b 48 8b 73 10 48 c7 c7 40 e0 2a 88 e8 43 27 f3 ff <0f> 0b e9 43 ff
ff ff e8 f7 7c 6c 00 e9 26 ff ff ff e8 ed 7c 6c 00
RSP: 0018:ffff8881b8db7948 EFLAGS: 00010286
RAX: 0000000000000000 RBX: ffff8881b8cae6c0 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff8164e045 RDI: 0000000000000005
RBP: ffff8881b8db7978 R08: ffff8881b8cae6c0 R09: 0000000000000007
R10: 0000000000000000 R11: ffff8881b8cae6c0 R12: ffffffff8860b220
R13: 0000000000000019 R14: 0000000000000000 R15: ffff8881b8cae6c0
__might_fault+0xc6/0x1e0 mm/memory.c:4357
_copy_to_user+0x30/0x110 lib/usercopy.c:25
copy_to_user include/linux/uaccess.h:155 [inline]
n_hdlc_tty_read+0x5c1/0x7f0 drivers/tty/n_hdlc.c:600
tty_read+0x194/0x2a0 drivers/tty/tty_io.c:866
__vfs_read+0x117/0x9b0 fs/read_write.c:416
vfs_read+0x17f/0x3c0 fs/read_write.c:452
ksys_read+0x101/0x260 fs/read_write.c:578
__do_sys_read fs/read_write.c:588 [inline]
__se_sys_read fs/read_write.c:586 [inline]
__x64_sys_read+0x73/0xb0 fs/read_write.c:586
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x440099
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffeeb4e9ec8 EFLAGS: 00000207 ORIG_RAX: 0000000000000000
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000440099
RDX: 0000000000000057 RSI: 0000000020000080 RDI: 0000000000000003
RBP: 00000000006ca018 R08: 00000000004002c8 R09: 00000000004002c8
R10: 00000000004002c8 R11: 0000000000000207 R12: 0000000000401920
R13: 00000000004019b0 R14: 0000000000000000 R15: 0000000000000000
Kernel Offset: disabled
Rebooting in 86400 seconds..


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzk...@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

[Tetsuo Handa]

syzbot is hitting __might_sleep() warning [1], for commit 1035b63d3c6fc34a
("n_hdlc: fix read and write locking") changed to set TASK_INTERRUPTIBLE
state before calling copy_to_user(). Let's set TASK_INTERRUPTIBLE state
immediately before calling schedule().

[1] https://syzkaller.appspot.com/bug?id=17d5de7f1fcab794cb8c40032f893f52de899324

Signed-off-by: Tetsuo Handa <penguin...@I-love.SAKURA.ne.jp>
Reported-by: syzbot <syzbot+c244af...@syzkaller.appspotmail.com>
Cc: Paul Fulghum <pau...@microgate.com>
Cc: Arnd Bergmann <ar...@arndb.de>
Cc: Alan Cox <al...@lxorguk.ukuu.org.uk>
---
drivers/tty/n_hdlc.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index dabb391..7835489 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -589,8 +589,6 @@ static ssize_t n_hdlc_tty_read(struct tty_struct *tty, struct file *file,
if (tty_hung_up_p(file))
break;

- set_current_state(TASK_INTERRUPTIBLE);
-
rbuf = n_hdlc_buf_get(&n_hdlc->rx_buf_list);
if (rbuf) {
if (rbuf->count > nr) {
@@ -617,6 +615,7 @@ static ssize_t n_hdlc_tty_read(struct tty_struct *tty, struct file *file,
break;
}

+ set_current_state(TASK_INTERRUPTIBLE);
schedule();

if (signal_pending(current)) {
@@ -673,8 +672,6 @@ static ssize_t n_hdlc_tty_write(struct tty_struct *tty, struct file *file,
add_wait_queue(&tty->write_wait, &wait);

for (;;) {
- set_current_state(TASK_INTERRUPTIBLE);
-
tbuf = n_hdlc_buf_get(&n_hdlc->tx_free_buf_list);
if (tbuf)
break;
@@ -683,6 +680,8 @@ static ssize_t n_hdlc_tty_write(struct tty_struct *tty, struct file *file,
error = -EAGAIN;
break;
}
+
+ set_current_state(TASK_INTERRUPTIBLE);
schedule();

n_hdlc = tty2n_hdlc (tty);
--
1.8.3.1

[Paul Fulghum]

NAK to this patch. It causes lost wakeups in both read and write paths.

The write path does not need changing.

The read path can be fixed by setting current to TASK_RUNNING at the top of the if (rbuf) block so the warning is not triggered by copy_to_user(). If this block runs the condition is satisfied and it breaks out of the polling loop where it is already being set to TASK_RUNNING and removed from the wait queue. This particular path just needs to account for the copy_to_user which occurs before breaking out.

I’ll make a patch to do this when I have the ability to test it in a day or two.

--

Paul Fulghum
MicroGate Systems, Ltd.
=Customer Driven, by Design=
(512) 345-7791 x102 (Voice)
(512) 343-9046 (Fax)
Central Time Zone (GMT -5h)
www.microgate.com

[Paul Fulghum]

On Dec 31, 2018, at 7:11 PM, Paul Fulghum <pau...@microgate.com> wrote:

NAK to this patch. It causes lost wakeups in both read and write paths.

The write path does not need changing.

The read path can be fixed by setting current to TASK_RUNNING at the top of the if (rbuf) block so the warning is not triggered by copy_to_user(). If this block runs the condition is satisfied and it breaks out of the polling loop where it is already being set to TASK_RUNNING and removed from the wait queue. This particular path just needs to account for the copy_to_user which occurs before breaking out.

I’ll make a patch to do this when I have the ability to test it in a day or two.


> On Dec 29, 2018, at 3:48 AM, Tetsuo Handa <penguin...@I-love.SAKURA.ne.jp> wrote:
>

[Paul Fulghum]

Fix __might_sleep warning in tty/n_hdlc.c read due to copy_to_user call while current is TASK_INTERRUPTIBLE.
This is a false positive since the code path does not depend on current state remaining TASK_INTERRUPTIBLE.
The loop breaks out and sets TASK_RUNNING after calling copy_to_user.
This patch supresses the warning by setting TASK_RUNNING before calling copy_to_user.

[1] https://syzkaller.appspot.com/bug?id=17d5de7f1fcab794cb8c40032f893f52de899324

Signed-off-by: Paul Fulghum <pau...@microgate.com>
Reported-by: syzbot <syzbot+c244af...@syzkaller.appspotmail.com>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: Tetsuo Handa <penguin...@I-love.SAKURA.ne.jp>

Cc: Arnd Bergmann <ar...@arndb.de>
Cc: Alan Cox <al...@lxorguk.ukuu.org.uk>
—

--- a/drivers/tty/n_hdlc.c 2018-12-23 15:55:59.000000000 -0800
+++ b/drivers/tty/n_hdlc.c 2019-01-01 11:44:47.148153954 -0800
@@ -597,6 +597,7 @@ static ssize_t n_hdlc_tty_read(struct tt
/* too large for caller's buffer */
ret = -EOVERFLOW;
} else {
+ __set_current_state(TASK_RUNNING);
if (copy_to_user(buf, rbuf->buf, rbuf->count))
ret = -EFAULT;
else

[Tetsuo Handa]

Hello, Greg.

We attempted a cleanup but we failed. Thus, will you apply this patch for now?

[Arnd Bergmann]

On Thu, Jan 10, 2019 at 12:39 PM Tetsuo Handa
<penguin...@i-love.sakura.ne.jp> wrote:
> On 2019/01/02 5:28, Paul Fulghum wrote:
> > Fix __might_sleep warning in tty/n_hdlc.c read due to copy_to_user call while current is TASK_INTERRUPTIBLE.
> > This is a false positive since the code path does not depend on current state remaining TASK_INTERRUPTIBLE.
> > The loop breaks out and sets TASK_RUNNING after calling copy_to_user.
> > This patch supresses the warning by setting TASK_RUNNING before calling copy_to_user.
> >
> > [1] https://syzkaller.appspot.com/bug?id=17d5de7f1fcab794cb8c40032f893f52de899324
> >
> > Signed-off-by: Paul Fulghum <pau...@microgate.com>
> > Reported-by: syzbot <syzbot+c244af...@syzkaller.appspotmail.com>
> > Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
> > Cc: Tetsuo Handa <penguin...@I-love.SAKURA.ne.jp>
> > Cc: Arnd Bergmann <ar...@arndb.de>
> > Cc: Alan Cox <al...@lxorguk.ukuu.org.uk>

This version looks good to me.

Acked-by: Arnd Bergmann <ar...@arndb.de>

Arnd