On 2020/11/14 2:14, Shuah Khan wrote: > On 11/13/20 5:00 AM, Hillf Danton wrote: >> Thu, 12 Nov 2020 23:21:26 -0800 >>> syzbot found the following issue on: >>> >>> HEAD commit: 9dbc1c03 Merge tag 'xfs-5.10-fixes-3' of git://git.kernel... >>> git tree: upstream >>> console output: https://syzkaller.appspot.com/x/log.txt?x=10453034500000 >>> kernel config: https://syzkaller.appspot.com/x/.config?x=1735b7978b1c3721 >>> dashboard link: https://syzkaller.appspot.com/bug?extid=95ce4b142579611ef0a9 >>> compiler: gcc (GCC) 10.1.0-syz 20200507 >>> userspace arch: i386 >>> >>> Unfortunately, I don't have any reproducer for this issue yet. >>> > > I would like to see the reproducer for this. I just can't reproduce > this problem. > >> >> Fix 96c2737716d5 ("usbip: move usbip kernel code out of staging") >> by closing the race between readers and writer of ud->tcp_socket on >> sock shutdown. To do that, add waitqueue for usbip device and make >> writer wait for all readers to go home before releasing the socket.
Worrysome part for me is vhci_device_reset() which resets ud->tcp_socket to NULL without waiting for tx thread to terminate, though I don't know if vhci_device_reset() can be called while tx thread is running. I'd like to try below debug printk() patch on linx-next tree, for this bug is reported on linux.git and linux-next.git trees. Which git tree can be used for sending this to-be-removed patch to linux-next.git ? I wish there is a kernel config for fuzzers in linux.git so that every git tree can carry debug printk() patch for syzbot's reports... Subject: [PATCH] usb: usbip: vhci_hcd: add printk() for debug This is linux-next only patch for examining a bug reported at https://syzkaller.appspot.com/bug?id=3e1d941a31361efc4ced2ba8b4af2044d4e43c59 . Signed-off-by: Tetsuo Handa <penguin-ker...@i-love.sakura.ne.jp> --- drivers/usb/usbip/stub_dev.c | 6 ++++++ drivers/usb/usbip/vhci_hcd.c | 11 +++++++++++ drivers/usb/usbip/vhci_sysfs.c | 4 ++++ drivers/usb/usbip/vhci_tx.c | 12 ++++++++++++ drivers/usb/usbip/vudc_dev.c | 6 ++++++ 5 files changed, 39 insertions(+) diff --git a/drivers/usb/usbip/stub_dev.c b/drivers/usb/usbip/stub_dev.c index 2305d425e6c9..627f83c0ebc8 100644 --- a/drivers/usb/usbip/stub_dev.c +++ b/drivers/usb/usbip/stub_dev.c @@ -131,10 +131,14 @@ static void stub_shutdown_connection(struct usbip_device *ud) /* 1. stop threads */ if (ud->tcp_rx) { + if (IS_BUILTIN(CONFIG_DEBUG_AID_FOR_SYZBOT)) + pr_info("%s: stop rx %p\n", __func__, ud->tcp_rx); kthread_stop_put(ud->tcp_rx); ud->tcp_rx = NULL; } if (ud->tcp_tx) { + if (IS_BUILTIN(CONFIG_DEBUG_AID_FOR_SYZBOT)) + pr_info("%s: stop tx %p\n", __func__, ud->tcp_tx); kthread_stop_put(ud->tcp_tx); ud->tcp_tx = NULL; } @@ -146,6 +150,8 @@ static void stub_shutdown_connection(struct usbip_device *ud) * not touch NULL socket. */ if (ud->tcp_socket) { + if (IS_BUILTIN(CONFIG_DEBUG_AID_FOR_SYZBOT)) + pr_info("%s: close sock %p\n", __func__, ud->tcp_socket); sockfd_put(ud->tcp_socket); ud->tcp_socket = NULL; ud->sockfd = -1; diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c index 3209b5ddd30c..9e95bf9330f5 100644 --- a/drivers/usb/usbip/vhci_hcd.c +++ b/drivers/usb/usbip/vhci_hcd.c @@ -1016,10 +1016,14 @@ static void vhci_shutdown_connection(struct usbip_device *ud) /* kill threads related to this sdev */ if (vdev->ud.tcp_rx) { + if (IS_BUILTIN(CONFIG_DEBUG_AID_FOR_SYZBOT)) + pr_info("%s: stop rx %p\n", __func__, vdev->ud.tcp_rx); kthread_stop_put(vdev->ud.tcp_rx); vdev->ud.tcp_rx = NULL; } if (vdev->ud.tcp_tx) { + if (IS_BUILTIN(CONFIG_DEBUG_AID_FOR_SYZBOT)) + pr_info("%s: stop tx %p\n", __func__, vdev->ud.tcp_tx); kthread_stop_put(vdev->ud.tcp_tx); vdev->ud.tcp_tx = NULL; } @@ -1027,6 +1031,8 @@ static void vhci_shutdown_connection(struct usbip_device *ud) /* active connection is closed */ if (vdev->ud.tcp_socket) { + if (IS_BUILTIN(CONFIG_DEBUG_AID_FOR_SYZBOT)) + pr_info("%s: close sock %p\n", __func__, ud->tcp_socket); sockfd_put(vdev->ud.tcp_socket); vdev->ud.tcp_socket = NULL; vdev->ud.sockfd = -1; @@ -1074,6 +1080,11 @@ static void vhci_device_reset(struct usbip_device *ud) vdev->udev = NULL; if (ud->tcp_socket) { + if (IS_BUILTIN(CONFIG_DEBUG_AID_FOR_SYZBOT)) { + pr_info("%s: close sock %p\n", __func__, ud->tcp_socket); + BUG_ON(vdev->ud.tcp_tx); + BUG_ON(vdev->ud.tcp_rx); + } sockfd_put(ud->tcp_socket); ud->tcp_socket = NULL; ud->sockfd = -1; diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c index be37aec250c2..b37e7770aa35 100644 --- a/drivers/usb/usbip/vhci_sysfs.c +++ b/drivers/usb/usbip/vhci_sysfs.c @@ -390,6 +390,10 @@ static ssize_t attach_store(struct device *dev, struct device_attribute *attr, vdev->ud.tcp_rx = kthread_get_run(vhci_rx_loop, &vdev->ud, "vhci_rx"); vdev->ud.tcp_tx = kthread_get_run(vhci_tx_loop, &vdev->ud, "vhci_tx"); + if (IS_BUILTIN(CONFIG_DEBUG_AID_FOR_SYZBOT)) { + BUG_ON(IS_ERR(vdev->ud.tcp_rx)); + BUG_ON(IS_ERR(vdev->ud.tcp_tx)); + } rh_port_connect(vdev, speed); diff --git a/drivers/usb/usbip/vhci_tx.c b/drivers/usb/usbip/vhci_tx.c index 0ae40a13a9fe..05da652e7bbe 100644 --- a/drivers/usb/usbip/vhci_tx.c +++ b/drivers/usb/usbip/vhci_tx.c @@ -63,6 +63,9 @@ static int vhci_send_cmd_submit(struct vhci_device *vdev) int iovnum; int err = -ENOMEM; int i; +#ifdef CONFIG_DEBUG_AID_FOR_SYZBOT + struct socket *socket = vdev->ud.tcp_socket; +#endif while ((priv = dequeue_from_priv_tx(vdev)) != NULL) { int ret; @@ -135,6 +138,11 @@ static int vhci_send_cmd_submit(struct vhci_device *vdev) iovnum++; txsize += len; } +#ifdef CONFIG_DEBUG_AID_FOR_SYZBOT + if (!socket || socket != vdev->ud.tcp_socket) + pr_err("%s: sock changed from %p to %p\n", + __func__, socket, vdev->ud.tcp_socket); +#endif ret = kernel_sendmsg(vdev->ud.tcp_socket, &msg, iov, iovnum, txsize); @@ -237,6 +245,8 @@ int vhci_tx_loop(void *data) struct usbip_device *ud = data; struct vhci_device *vdev = container_of(ud, struct vhci_device, ud); + if (IS_BUILTIN(CONFIG_DEBUG_AID_FOR_SYZBOT)) + pr_info("%s: thread starting %p\n", __func__, vdev->ud.tcp_tx); while (!kthread_should_stop()) { if (vhci_send_cmd_submit(vdev) < 0) break; @@ -251,6 +261,8 @@ int vhci_tx_loop(void *data) usbip_dbg_vhci_tx("pending urbs ?, now wake up\n"); } + if (IS_BUILTIN(CONFIG_DEBUG_AID_FOR_SYZBOT)) + pr_info("%s: thread exiting %p\n", __func__, vdev->ud.tcp_tx); return 0; } diff --git a/drivers/usb/usbip/vudc_dev.c b/drivers/usb/usbip/vudc_dev.c index c8eeabdd9b56..816cb4b5700b 100644 --- a/drivers/usb/usbip/vudc_dev.c +++ b/drivers/usb/usbip/vudc_dev.c @@ -437,15 +437,21 @@ static void vudc_shutdown(struct usbip_device *ud) kernel_sock_shutdown(ud->tcp_socket, SHUT_RDWR); if (ud->tcp_rx) { + if (IS_BUILTIN(CONFIG_DEBUG_AID_FOR_SYZBOT)) + pr_info("%s: stop rx %p\n", __func__, ud->tcp_rx); kthread_stop_put(ud->tcp_rx); ud->tcp_rx = NULL; } if (ud->tcp_tx) { + if (IS_BUILTIN(CONFIG_DEBUG_AID_FOR_SYZBOT)) + pr_info("%s: stop tx %p\n", __func__, ud->tcp_tx); kthread_stop_put(ud->tcp_tx); ud->tcp_tx = NULL; } if (ud->tcp_socket) { + if (IS_BUILTIN(CONFIG_DEBUG_AID_FOR_SYZBOT)) + pr_info("%s: close sock %p\n", __func__, ud->tcp_socket); sockfd_put(ud->tcp_socket); ud->tcp_socket = NULL; } -- 2.18.4