All of lore.kernel.org
 help / color / mirror / Atom feed
* INFO: task hung in nbd_ioctl (2)
@ 2020-07-06 15:22 syzbot
  2020-09-02 11:09 ` [PATCH] tipc: fix shutdown() of connectionless socket Tetsuo Handa
  0 siblings, 1 reply; 9+ messages in thread
From: syzbot @ 2020-07-06 15:22 UTC (permalink / raw)
  To: axboe, josef, linux-block, linux-kernel, nbd, syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    7ae77150 Merge tag 'powerpc-5.8-1' of git://git.kernel.org..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1124dead100000
kernel config:  https://syzkaller.appspot.com/x/.config?x=d195fe572fb15312
dashboard link: https://syzkaller.appspot.com/bug?extid=e36f41d207137b5d12f7
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)

Unfortunately, I don't have any reproducer for this crash yet.

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

INFO: task syz-executor.1:13143 blocked for more than 143 seconds.
      Not tainted 5.7.0-syzkaller #0
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
syz-executor.1  D27616 13143   6943 0x00004004
Call Trace:
 context_switch kernel/sched/core.c:3430 [inline]
 __schedule+0x8f3/0x1fc0 kernel/sched/core.c:4156
 schedule+0xd0/0x2a0 kernel/sched/core.c:4231
 schedule_timeout+0x55b/0x850 kernel/time/timer.c:1873
 do_wait_for_common kernel/sched/completion.c:85 [inline]
 __wait_for_common kernel/sched/completion.c:106 [inline]
 wait_for_common kernel/sched/completion.c:117 [inline]
 wait_for_completion+0x16a/0x270 kernel/sched/completion.c:138
 flush_workqueue+0x403/0x14f0 kernel/workqueue.c:2831
 nbd_start_device_ioctl drivers/block/nbd.c:1322 [inline]
 __nbd_ioctl drivers/block/nbd.c:1397 [inline]
 nbd_ioctl+0x7ae/0xb7f drivers/block/nbd.c:1437
 __blkdev_driver_ioctl block/ioctl.c:224 [inline]
 blkdev_ioctl+0x25e/0x6c0 block/ioctl.c:620
 block_ioctl+0xf9/0x140 fs/block_dev.c:1987
 vfs_ioctl fs/ioctl.c:47 [inline]
 ksys_ioctl+0x11a/0x180 fs/ioctl.c:771
 __do_sys_ioctl fs/ioctl.c:780 [inline]
 __se_sys_ioctl fs/ioctl.c:778 [inline]
 __x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:778
 do_syscall_64+0xf6/0x7d0 arch/x86/entry/common.c:295
 entry_SYSCALL_64_after_hwframe+0x49/0xb3
RIP: 0033:0x45ca59
Code: Bad RIP value.
RSP: 002b:00007fc5b74b2c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00000000004e98c0 RCX: 000000000045ca59
RDX: 0000000000000000 RSI: 000000000000ab03 RDI: 0000000000000003
RBP: 000000000078bf00 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
R13: 0000000000000415 R14: 00000000004c6d6f R15: 00007fc5b74b36d4

Showing all locks held in the system:
1 lock held by khungtaskd/1147:
 #0: ffffffff899bdd80 (rcu_read_lock){....}-{1:2}, at: debug_show_all_locks+0x53/0x260 kernel/locking/lockdep.c:5779
2 locks held by kworker/u5:0/1521:
 #0: ffff888097167138 ((wq_completion)knbd1-recv){+.+.}-{0:0}, at: __write_once_size include/linux/compiler.h:279 [inline]
 #0: ffff888097167138 ((wq_completion)knbd1-recv){+.+.}-{0:0}, at: arch_atomic64_set arch/x86/include/asm/atomic64_64.h:34 [inline]
 #0: ffff888097167138 ((wq_completion)knbd1-recv){+.+.}-{0:0}, at: atomic64_set include/asm-generic/atomic-instrumented.h:855 [inline]
 #0: ffff888097167138 ((wq_completion)knbd1-recv){+.+.}-{0:0}, at: atomic_long_set include/asm-generic/atomic-long.h:40 [inline]
 #0: ffff888097167138 ((wq_completion)knbd1-recv){+.+.}-{0:0}, at: set_work_data kernel/workqueue.c:615 [inline]
 #0: ffff888097167138 ((wq_completion)knbd1-recv){+.+.}-{0:0}, at: set_work_pool_and_clear_pending kernel/workqueue.c:642 [inline]
 #0: ffff888097167138 ((wq_completion)knbd1-recv){+.+.}-{0:0}, at: process_one_work+0x844/0x16a0 kernel/workqueue.c:2239
 #1: ffffc90005357dc0 ((work_completion)(&args->work)){+.+.}-{0:0}, at: process_one_work+0x878/0x16a0 kernel/workqueue.c:2243
1 lock held by in:imklog/6476:
2 locks held by agetty/6489:
 #0: ffff88809ebb3098 (&tty->ldisc_sem){++++}-{0:0}, at: tty_ldisc_ref_wait+0x22/0x80 drivers/tty/tty_ldisc.c:267
 #1: ffffc90000eb42e8 (&ldata->atomic_read_lock){+.+.}-{3:3}, at: n_tty_read+0x220/0x1b30 drivers/tty/n_tty.c:2156

=============================================

NMI backtrace for cpu 0
CPU: 0 PID: 1147 Comm: khungtaskd Not tainted 5.7.0-syzkaller #0
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+0x188/0x20d lib/dump_stack.c:118
 nmi_cpu_backtrace.cold+0x70/0xb1 lib/nmi_backtrace.c:101
 nmi_trigger_cpumask_backtrace+0x1e6/0x221 lib/nmi_backtrace.c:62
 trigger_all_cpu_backtrace include/linux/nmi.h:146 [inline]
 check_hung_uninterruptible_tasks kernel/hung_task.c:205 [inline]
 watchdog+0xa8c/0x1010 kernel/hung_task.c:289
 kthread+0x388/0x470 kernel/kthread.c:268
 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:351
Sending NMI from CPU 0 to CPUs 1:
NMI backtrace for cpu 1
CPU: 1 PID: 3858 Comm: systemd-journal Not tainted 5.7.0-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:getname_flags fs/namei.c:150 [inline]
RIP: 0010:getname_flags+0x122/0x5b0 fs/namei.c:128
Code: 03 80 3c 02 00 0f 85 be 01 00 00 4c 89 f7 4d 89 34 24 ba e0 0f 00 00 48 89 de e8 e9 f1 ec 01 31 ff 49 89 c6 89 c6 89 44 24 04 <e8> a9 67 b2 ff 45 85 f6 0f 88 9e 01 00 00 e8 fb 65 b2 ff 44 89 f6
RSP: 0018:ffffc90001627df8 EFLAGS: 00000246
RAX: 000000000000001c RBX: 000055e1d31c89a3 RCX: ffffffff83ae5c52
RDX: 0000000000000000 RSI: 000000000000001c RDI: 0000000000000000
RBP: 0000000000000001 R08: ffff888094f5a480 R09: ffff8880aa1ec000
R10: 000000000000001c R11: fffff9400053f070 R12: ffff8880a7e0e940
R13: 0000000000000000 R14: 000000000000001c R15: ffffffff8a8b9b48
FS:  00007fb1526cd8c0(0000) GS:ffff8880ae700000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fb14fb8c000 CR3: 0000000094814000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 user_path_at_empty+0x2a/0x50 fs/namei.c:2632
 user_path_at include/linux/namei.h:59 [inline]
 do_faccessat+0x12c/0x830 fs/open.c:423
 do_syscall_64+0xf6/0x7d0 arch/x86/entry/common.c:295
 entry_SYSCALL_64_after_hwframe+0x49/0xb3
RIP: 0033:0x7fb1519899c7
Code: 83 c4 08 48 3d 01 f0 ff ff 73 01 c3 48 8b 0d c8 d4 2b 00 f7 d8 64 89 01 48 83 c8 ff c3 66 0f 1f 44 00 00 b8 15 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d a1 d4 2b 00 f7 d8 64 89 01 48
RSP: 002b:00007ffe422e0be8 EFLAGS: 00000246 ORIG_RAX: 0000000000000015
RAX: ffffffffffffffda RBX: 00007ffe422e3b00 RCX: 00007fb1519899c7
RDX: 00007fb1523faa00 RSI: 0000000000000000 RDI: 000055e1d31c89a3
RBP: 00007ffe422e0c20 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000069 R11: 0000000000000246 R12: 0000000000000000
R13: 0000000000000000 R14: 00007ffe422e3b00 R15: 00007ffe422e1110


---
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 syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] tipc: fix shutdown() of connectionless socket
  2020-07-06 15:22 INFO: task hung in nbd_ioctl (2) syzbot
@ 2020-09-02 11:09 ` Tetsuo Handa
  2020-09-02 13:44   ` [PATCH v2] " Tetsuo Handa
  2020-09-03 11:31   ` [PATCH] " Wouter Verhelst
  0 siblings, 2 replies; 9+ messages in thread
From: Tetsuo Handa @ 2020-09-02 11:09 UTC (permalink / raw)
  To: syzbot, Jon Maloy, Ying Xue
  Cc: syzkaller-bugs, David S. Miller, Jakub Kicinski, netdev, tipc-discussion

syzbot is reporting hung task at nbd_ioctl() [1], for there are two
problems regarding TIPC's connectionless socket's shutdown() operation.
I found C reproducer for this problem (shown below) from "no output from
test machine (2)" report.

----------

int main(int argc, char *argv[])
{
        const int fd = open("/dev/nbd0", 3);
        ioctl(fd, NBD_SET_SOCK, socket(PF_TIPC, SOCK_DGRAM, 0));
        ioctl(fd, NBD_DO_IT, 0);
        return 0;
}
----------

One problem is that wait_for_completion() from flush_workqueue() from
nbd_start_device_ioctl() from nbd_ioctl() cannot be completed when
nbd_start_device_ioctl() received a signal at wait_event_interruptible(),
for tipc_shutdown() from kernel_sock_shutdown(SHUT_RDWR) from
nbd_mark_nsock_dead() from sock_shutdown() from nbd_start_device_ioctl()
is failing to wake up a WQ thread sleeping at wait_woken() from
tipc_wait_for_rcvmsg() from sock_recvmsg() from sock_xmit() from
nbd_read_stat() from recv_work() scheduled by nbd_start_device() from
nbd_start_device_ioctl(). Fix this problem by always invoking
sk->sk_state_change() (like inet_shutdown() does) when tipc_shutdown() is
called.

The other problem is that tipc_wait_for_rcvmsg() cannot return when
tipc_shutdown() is called, for tipc_shutdown() sets sk->sk_shutdown to
SEND_SHUTDOWN (despite "how" is SHUT_RDWR) while tipc_wait_for_rcvmsg()
needs sk->sk_shutdown set to RCV_SHUTDOWN or SHUTDOWN_MASK. Fix this
problem by setting sk->sk_shutdown to SHUTDOWN_MASK (like inet_shutdown()
does) when the socket is connectionless.

[1] https://syzkaller.appspot.com/bug?id=3fe51d307c1f0a845485cf1798aa059d12bf18b2

Reported-by: syzbot <syzbot+e36f41d207137b5d12f7@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 net/tipc/socket.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 2679e97e0389..ebd280e767bd 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -2771,18 +2771,21 @@ static int tipc_shutdown(struct socket *sock, int how)
 
 	trace_tipc_sk_shutdown(sk, NULL, TIPC_DUMP_ALL, " ");
 	__tipc_shutdown(sock, TIPC_CONN_SHUTDOWN);
-	sk->sk_shutdown = SEND_SHUTDOWN;
+	if (tipc_sk_type_connectionless(sk))
+		sk->sk_shutdown = SHUTDOWN_MASK;
+	else
+		sk->sk_shutdown = SEND_SHUTDOWN;
 
 	if (sk->sk_state == TIPC_DISCONNECTING) {
 		/* Discard any unreceived messages */
 		__skb_queue_purge(&sk->sk_receive_queue);
 
-		/* Wake up anyone sleeping in poll */
-		sk->sk_state_change(sk);
 		res = 0;
 	} else {
 		res = -ENOTCONN;
 	}
+	/* Wake up anyone sleeping in poll. */
+	sk->sk_state_change(sk);
 
 	release_sock(sk);
 	return res;
-- 
2.18.4



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2] tipc: fix shutdown() of connectionless socket
  2020-09-02 11:09 ` [PATCH] tipc: fix shutdown() of connectionless socket Tetsuo Handa
@ 2020-09-02 13:44   ` Tetsuo Handa
  2020-09-02 22:50     ` David Miller
  2020-09-03 11:31   ` [PATCH] " Wouter Verhelst
  1 sibling, 1 reply; 9+ messages in thread
From: Tetsuo Handa @ 2020-09-02 13:44 UTC (permalink / raw)
  To: syzbot, Jon Maloy, Ying Xue
  Cc: syzkaller-bugs, David S. Miller, Jakub Kicinski, netdev, tipc-discussion

syzbot is reporting hung task at nbd_ioctl() [1], for there are two
problems regarding TIPC's connectionless socket's shutdown() operation.

----------
#include <fcntl.h>
#include <sys/socket.h>
#include <sys/ioctl.h>
#include <linux/nbd.h>
#include <unistd.h>

int main(int argc, char *argv[])
{
        const int fd = open("/dev/nbd0", 3);
        alarm(5);
        ioctl(fd, NBD_SET_SOCK, socket(PF_TIPC, SOCK_DGRAM, 0));
        ioctl(fd, NBD_DO_IT, 0); /* To be interrupted by SIGALRM. */
        return 0;
}
----------

One problem is that wait_for_completion() from flush_workqueue() from
nbd_start_device_ioctl() from nbd_ioctl() cannot be completed when
nbd_start_device_ioctl() received a signal at wait_event_interruptible(),
for tipc_shutdown() from kernel_sock_shutdown(SHUT_RDWR) from
nbd_mark_nsock_dead() from sock_shutdown() from nbd_start_device_ioctl()
is failing to wake up a WQ thread sleeping at wait_woken() from
tipc_wait_for_rcvmsg() from sock_recvmsg() from sock_xmit() from
nbd_read_stat() from recv_work() scheduled by nbd_start_device() from
nbd_start_device_ioctl(). Fix this problem by always invoking
sk->sk_state_change() (like inet_shutdown() does) when tipc_shutdown() is
called.

The other problem is that tipc_wait_for_rcvmsg() cannot return when
tipc_shutdown() is called, for tipc_shutdown() sets sk->sk_shutdown to
SEND_SHUTDOWN (despite "how" is SHUT_RDWR) while tipc_wait_for_rcvmsg()
needs sk->sk_shutdown set to RCV_SHUTDOWN or SHUTDOWN_MASK. Fix this
problem by setting sk->sk_shutdown to SHUTDOWN_MASK (like inet_shutdown()
does) when the socket is connectionless.

[1] https://syzkaller.appspot.com/bug?id=3fe51d307c1f0a845485cf1798aa059d12bf18b2

Reported-by: syzbot <syzbot+e36f41d207137b5d12f7@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 net/tipc/socket.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 2679e97e0389..ebd280e767bd 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -2771,18 +2771,21 @@ static int tipc_shutdown(struct socket *sock, int how)
 
 	trace_tipc_sk_shutdown(sk, NULL, TIPC_DUMP_ALL, " ");
 	__tipc_shutdown(sock, TIPC_CONN_SHUTDOWN);
-	sk->sk_shutdown = SEND_SHUTDOWN;
+	if (tipc_sk_type_connectionless(sk))
+		sk->sk_shutdown = SHUTDOWN_MASK;
+	else
+		sk->sk_shutdown = SEND_SHUTDOWN;
 
 	if (sk->sk_state == TIPC_DISCONNECTING) {
 		/* Discard any unreceived messages */
 		__skb_queue_purge(&sk->sk_receive_queue);
 
-		/* Wake up anyone sleeping in poll */
-		sk->sk_state_change(sk);
 		res = 0;
 	} else {
 		res = -ENOTCONN;
 	}
+	/* Wake up anyone sleeping in poll. */
+	sk->sk_state_change(sk);
 
 	release_sock(sk);
 	return res;
-- 
2.18.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] tipc: fix shutdown() of connectionless socket
  2020-09-02 13:44   ` [PATCH v2] " Tetsuo Handa
@ 2020-09-02 22:50     ` David Miller
  2020-09-03 14:24       ` Tetsuo Handa
  0 siblings, 1 reply; 9+ messages in thread
From: David Miller @ 2020-09-02 22:50 UTC (permalink / raw)
  To: penguin-kernel
  Cc: syzbot+e36f41d207137b5d12f7, jmaloy, ying.xue, syzkaller-bugs,
	kuba, netdev, tipc-discussion

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Wed, 2 Sep 2020 22:44:16 +0900

> syzbot is reporting hung task at nbd_ioctl() [1], for there are two
> problems regarding TIPC's connectionless socket's shutdown() operation.
 ...
> One problem is that wait_for_completion() from flush_workqueue() from
> nbd_start_device_ioctl() from nbd_ioctl() cannot be completed when
> nbd_start_device_ioctl() received a signal at wait_event_interruptible(),
> for tipc_shutdown() from kernel_sock_shutdown(SHUT_RDWR) from
> nbd_mark_nsock_dead() from sock_shutdown() from nbd_start_device_ioctl()
> is failing to wake up a WQ thread sleeping at wait_woken() from
> tipc_wait_for_rcvmsg() from sock_recvmsg() from sock_xmit() from
> nbd_read_stat() from recv_work() scheduled by nbd_start_device() from
> nbd_start_device_ioctl(). Fix this problem by always invoking
> sk->sk_state_change() (like inet_shutdown() does) when tipc_shutdown() is
> called.
> 
> The other problem is that tipc_wait_for_rcvmsg() cannot return when
> tipc_shutdown() is called, for tipc_shutdown() sets sk->sk_shutdown to
> SEND_SHUTDOWN (despite "how" is SHUT_RDWR) while tipc_wait_for_rcvmsg()
> needs sk->sk_shutdown set to RCV_SHUTDOWN or SHUTDOWN_MASK. Fix this
> problem by setting sk->sk_shutdown to SHUTDOWN_MASK (like inet_shutdown()
> does) when the socket is connectionless.
> 
> [1] https://syzkaller.appspot.com/bug?id=3fe51d307c1f0a845485cf1798aa059d12bf18b2
> 
> Reported-by: syzbot <syzbot+e36f41d207137b5d12f7@syzkaller.appspotmail.com>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

Applied and queued up for -stable, thank you.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] tipc: fix shutdown() of connectionless socket
  2020-09-02 11:09 ` [PATCH] tipc: fix shutdown() of connectionless socket Tetsuo Handa
  2020-09-02 13:44   ` [PATCH v2] " Tetsuo Handa
@ 2020-09-03 11:31   ` Wouter Verhelst
  2020-09-03 11:57     ` Tetsuo Handa
  1 sibling, 1 reply; 9+ messages in thread
From: Wouter Verhelst @ 2020-09-03 11:31 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: syzbot, Jon Maloy, Ying Xue, syzkaller-bugs, David S. Miller,
	Jakub Kicinski, netdev, tipc-discussion

So.

On Wed, Sep 02, 2020 at 08:09:54PM +0900, Tetsuo Handa wrote:
> syzbot is reporting hung task at nbd_ioctl() [1], for there are two
> problems regarding TIPC's connectionless socket's shutdown() operation.
> I found C reproducer for this problem (shown below) from "no output from
> test machine (2)" report.
> 
> ----------
> 
> int main(int argc, char *argv[])
> {
>         const int fd = open("/dev/nbd0", 3);
>         ioctl(fd, NBD_SET_SOCK, socket(PF_TIPC, SOCK_DGRAM, 0));

NBD expects a stream socket, not a datagram one.

>         ioctl(fd, NBD_DO_IT, 0);

This is supposed to sit and wait until someone disconnects the device
again (which you probably cna't do with datagram sockets). Changing that
changes a userspace API.

-- 
To the thief who stole my anti-depressants: I hope you're happy

  -- seen somewhere on the Internet on a photo of a billboard

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] tipc: fix shutdown() of connectionless socket
  2020-09-03 11:31   ` [PATCH] " Wouter Verhelst
@ 2020-09-03 11:57     ` Tetsuo Handa
  2020-09-03 12:05       ` Wouter Verhelst
  0 siblings, 1 reply; 9+ messages in thread
From: Tetsuo Handa @ 2020-09-03 11:57 UTC (permalink / raw)
  To: Wouter Verhelst
  Cc: syzbot, Jon Maloy, Ying Xue, syzkaller-bugs, David S. Miller,
	Jakub Kicinski, netdev, tipc-discussion

On 2020/09/03 20:31, Wouter Verhelst wrote:
> So.
> 
> On Wed, Sep 02, 2020 at 08:09:54PM +0900, Tetsuo Handa wrote:
>> syzbot is reporting hung task at nbd_ioctl() [1], for there are two
>> problems regarding TIPC's connectionless socket's shutdown() operation.
>> I found C reproducer for this problem (shown below) from "no output from
>> test machine (2)" report.
>>
>> ----------
>>
>> int main(int argc, char *argv[])
>> {
>>         const int fd = open("/dev/nbd0", 3);
>>         ioctl(fd, NBD_SET_SOCK, socket(PF_TIPC, SOCK_DGRAM, 0));
> 
> NBD expects a stream socket, not a datagram one.
> 
>>         ioctl(fd, NBD_DO_IT, 0);
> 
> This is supposed to sit and wait until someone disconnects the device
> again (which you probably cna't do with datagram sockets). Changing that
> changes a userspace API.
> 

Excuse me, but other datagram sockets (e.g. socket(PF_INET, SOCK_DGRAM, 0)) does not
hit this problem. What do you want to do? Add a "whether the file descriptor passed
to ioctl(NBD_SET_SOCK) is a SOCK_STREAM socket" test to the NBD side?

I think that regardless of whether NBD expects only SOCK_STREAM sockets,
tipc_wait_for_rcvmsg() on a SOCK_DGRAM socket can't return is a bug.
David Miller already applied this patch and queued up for -stable.
Do we need to revert this patch?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] tipc: fix shutdown() of connectionless socket
  2020-09-03 11:57     ` Tetsuo Handa
@ 2020-09-03 12:05       ` Wouter Verhelst
  2020-09-03 19:11         ` David Miller
  0 siblings, 1 reply; 9+ messages in thread
From: Wouter Verhelst @ 2020-09-03 12:05 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: syzbot, Jon Maloy, Ying Xue, syzkaller-bugs, David S. Miller,
	Jakub Kicinski, netdev, tipc-discussion

On Thu, Sep 03, 2020 at 08:57:01PM +0900, Tetsuo Handa wrote:
> On 2020/09/03 20:31, Wouter Verhelst wrote:
> > So.
> > 
> > On Wed, Sep 02, 2020 at 08:09:54PM +0900, Tetsuo Handa wrote:
> >> syzbot is reporting hung task at nbd_ioctl() [1], for there are two
> >> problems regarding TIPC's connectionless socket's shutdown() operation.
> >> I found C reproducer for this problem (shown below) from "no output from
> >> test machine (2)" report.
> >>
> >> ----------
> >>
> >> int main(int argc, char *argv[])
> >> {
> >>         const int fd = open("/dev/nbd0", 3);
> >>         ioctl(fd, NBD_SET_SOCK, socket(PF_TIPC, SOCK_DGRAM, 0));
> > 
> > NBD expects a stream socket, not a datagram one.
> > 
> >>         ioctl(fd, NBD_DO_IT, 0);
> > 
> > This is supposed to sit and wait until someone disconnects the device
> > again (which you probably cna't do with datagram sockets). Changing that
> > changes a userspace API.
> > 
> 
> Excuse me, but other datagram sockets (e.g. socket(PF_INET, SOCK_DGRAM, 0)) does not
> hit this problem. What do you want to do? Add a "whether the file descriptor passed
> to ioctl(NBD_SET_SOCK) is a SOCK_STREAM socket" test to the NBD side?

I missed originally that you were checking whether the passed socket is
in fact a SOCK_DGRAM socket, and limiting the changes to that. That's
fine, because NBD doesn't deal with SOCK_DGRAM sockets anyway (i.e.,
passing a SOCK_DGRAM socket to the NBD device is undefined behavior). If
the behavior also changes for SOCK_STREAM sockets then that would be a
problem that would need to be reverted, but otherwise it's fine.

-- 
To the thief who stole my anti-depressants: I hope you're happy

  -- seen somewhere on the Internet on a photo of a billboard

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] tipc: fix shutdown() of connectionless socket
  2020-09-02 22:50     ` David Miller
@ 2020-09-03 14:24       ` Tetsuo Handa
  0 siblings, 0 replies; 9+ messages in thread
From: Tetsuo Handa @ 2020-09-03 14:24 UTC (permalink / raw)
  To: Parthasarathy Bhuvaragan
  Cc: David Miller, syzbot+e36f41d207137b5d12f7, jmaloy, ying.xue,
	syzkaller-bugs, kuba, netdev, tipc-discussion, Wouter Verhelst

Hello, Parthasarathy.

I have a question regarding commit 6f00089c7372ba97 ("tipc: remove SS_DISCONNECTING state").
That commit added

	sk->sk_shutdown = SEND_SHUTDOWN;

into tipc_shutdown(). What is the reason you chose SEND_SHUTDOWN despite how == SHUT_RDWR ?

Since Wouter commented that NBD expects SOCK_STREAM sockets, I think that passing TIPC's
stream socket is legal. And I can trigger hung task warning using a reproducer shown below.

----------
#include <fcntl.h>
#include <sys/socket.h>
#include <sys/ioctl.h>
#include <linux/nbd.h>
#include <unistd.h>

int main(int argc, char *argv[])
{
        const int fd = open("/dev/nbd0", 3);
        int fds[2] = { -1, -1 };
        alarm(5);
        socketpair(PF_TIPC, SOCK_STREAM, 0, fds);
        ioctl(fd, NBD_SET_SOCK, fds[0]);
        ioctl(fd, NBD_DO_IT, 0); /* To be interrupted by SIGALRM. */
        return 0;
}
----------

Applying a patch shown below solves the hung task warning, but I can't evaluate
the side effect of this patch, for I don't know why you chose SEND_SHUTDOWN and
how TIPC socket works. Can we apply this patch?

----------
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -2771,10 +2771,7 @@ static int tipc_shutdown(struct socket *sock, int how)
 
 	trace_tipc_sk_shutdown(sk, NULL, TIPC_DUMP_ALL, " ");
 	__tipc_shutdown(sock, TIPC_CONN_SHUTDOWN);
-	if (tipc_sk_type_connectionless(sk))
-		sk->sk_shutdown = SHUTDOWN_MASK;
-	else
-		sk->sk_shutdown = SEND_SHUTDOWN;
+	sk->sk_shutdown = SHUTDOWN_MASK;
 
 	if (sk->sk_state == TIPC_DISCONNECTING) {
 		/* Discard any unreceived messages */
----------

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] tipc: fix shutdown() of connectionless socket
  2020-09-03 12:05       ` Wouter Verhelst
@ 2020-09-03 19:11         ` David Miller
  0 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2020-09-03 19:11 UTC (permalink / raw)
  To: w
  Cc: penguin-kernel, syzbot+e36f41d207137b5d12f7, jmaloy, ying.xue,
	syzkaller-bugs, kuba, netdev, tipc-discussion

From: Wouter Verhelst <w@uter.be>
Date: Thu, 3 Sep 2020 14:05:15 +0200

> That's fine, because NBD doesn't deal with SOCK_DGRAM sockets anyway
> (i.e., passing a SOCK_DGRAM socket to the NBD device is undefined
> behavior).

Then why doesn't NBD simply reject such sockets?


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-09-03 19:11 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-06 15:22 INFO: task hung in nbd_ioctl (2) syzbot
2020-09-02 11:09 ` [PATCH] tipc: fix shutdown() of connectionless socket Tetsuo Handa
2020-09-02 13:44   ` [PATCH v2] " Tetsuo Handa
2020-09-02 22:50     ` David Miller
2020-09-03 14:24       ` Tetsuo Handa
2020-09-03 11:31   ` [PATCH] " Wouter Verhelst
2020-09-03 11:57     ` Tetsuo Handa
2020-09-03 12:05       ` Wouter Verhelst
2020-09-03 19:11         ` David Miller

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.