All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
To: Shuah Khan <skhan@linuxfoundation.org>,
	Hillf Danton <hdanton@sina.com>,
	syzbot <syzbot+95ce4b142579611ef0a9@syzkaller.appspotmail.com>
Cc: linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	Andrey Konovalov <andreyknvl@google.com>,
	Valentina Manea <valentina.manea.m@gmail.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	syzkaller-bugs@googlegroups.com
Subject: Re: general protection fault in tomoyo_socket_sendmsg_permission
Date: Thu, 28 Jan 2021 15:09:14 +0900	[thread overview]
Message-ID: <ea4028b7-53f2-aeaf-76e7-69874efcdec5@I-love.SAKURA.ne.jp> (raw)
In-Reply-To: <5f71e0c1-d387-6d72-d8e4-edb11cf57f72@linuxfoundation.org>

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-kernel@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

  reply	other threads:[~2021-01-28  6:11 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-13  7:21 general protection fault in tomoyo_socket_sendmsg_permission syzbot
2020-11-13 10:49 ` Tetsuo Handa
     [not found] ` <20201113120055.11748-1-hdanton@sina.com>
2020-11-13 17:14   ` Shuah Khan
2021-01-28  6:09     ` Tetsuo Handa [this message]
     [not found]       ` <2b70d360-a293-4acb-ea6c-2badda5e8b8b@linuxfoundation.org>
2021-01-29  5:48         ` Tetsuo Handa
     [not found]           ` <6b8da36f-a994-7604-77f4-52e29434605f@linuxfoundation.org>
2021-01-29 17:08             ` Tetsuo Handa
2021-01-29 21:18               ` Shuah Khan
2021-01-30  2:25                 ` Tetsuo Handa
2021-02-10 18:17                   ` Shuah Khan
2021-02-10 19:07                     ` Tetsuo Handa
2021-02-10 19:29                       ` Shuah Khan
2021-02-11  1:14                         ` Tetsuo Handa
2021-02-12  1:34                           ` Shuah Khan
2021-02-12  2:22                             ` Tetsuo Handa
2021-02-12  4:58                               ` Tetsuo Handa
2021-02-12 20:02                                 ` Shuah Khan
2021-02-13 10:02                                   ` Tetsuo Handa
2021-02-13 10:10                                     ` Greg Kroah-Hartman
2021-02-13 10:10                                     ` Greg Kroah-Hartman
2021-02-19  0:33 ` [PATCH] usb: usbip: serialize attach/detach operations Tetsuo Handa
2021-02-19  9:47   ` [PATCH (repost)] " Tetsuo Handa
2021-02-19 15:08     ` [PATCH v2] " Tetsuo Handa
2021-02-19 15:53       ` Greg Kroah-Hartman
2021-02-19 16:00         ` Shuah Khan
2021-02-20  1:10           ` Tetsuo Handa
2021-02-20  6:58             ` Greg Kroah-Hartman
2021-02-20  9:51               ` Tetsuo Handa
2021-02-22 15:34                 ` Shuah Khan
2021-02-23  1:51                   ` Tetsuo Handa
2021-02-23  1:59               ` [PATCH v3] " Tetsuo Handa
2021-02-26  0:00                 ` Shuah Khan
2021-02-26  0:10                   ` Tetsuo Handa
2021-02-26 15:04                     ` Shuah Khan
2021-03-04 15:24                       ` [PATCH v4 00/12] " Tetsuo Handa
2021-03-04 15:24                         ` [PATCH v4 01/12] usb: usbip: introduce usbip_event_mutex for serialization Tetsuo Handa
2021-03-04 15:24                         ` [PATCH v4 02/12] usb: usbip: vhci: serialize attach_store()/detach_store() against event_handler() Tetsuo Handa
2021-03-04 15:24                         ` [PATCH v4 03/12] usb: usbip: vudc: serialize usbip_sockfd_store() " Tetsuo Handa
2021-03-04 15:24                         ` [PATCH v4 04/12] usb: usbip: stub: " Tetsuo Handa
2021-03-04 15:24                         ` [PATCH v4 05/12] usb: usbip: don't reset tcp_socket at vhci_device_reset() Tetsuo Handa
2021-03-04 15:24                         ` [PATCH v4 06/12] usb: usbip: fix error handling of kthread_get_run() Tetsuo Handa
2021-03-04 15:24                         ` [PATCH v4 07/12] usb: usbip: preallocate kernel threads for consistent attach operation Tetsuo Handa
2021-03-04 15:24                         ` [PATCH v4 08/12] usb: usbip: check that stream socket is used Tetsuo Handa
2021-03-04 15:24                         ` [PATCH v4 09/12] usb: usbip: vhci: add automatic recovery to attach_store() Tetsuo Handa
2021-03-04 15:24                         ` [PATCH v4 10/12] usb: usbip: vudc: add automatic recovery to usbip_sockfd_store() Tetsuo Handa
2021-03-04 15:24                         ` [PATCH v4 11/12] usb: usbip: stub: " Tetsuo Handa
2021-03-04 15:24                         ` [PATCH v4 12/12] usb: usbip: remove unused kthread_get_run() Tetsuo Handa
2021-03-04 15:52                         ` [PATCH v4 00/12] usb: usbip: serialize attach/detach operations Shuah Khan
2021-03-05 10:06                           ` Tetsuo Handa
2021-03-05 14:44                             ` Shuah Khan

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=ea4028b7-53f2-aeaf-76e7-69874efcdec5@I-love.SAKURA.ne.jp \
    --to=penguin-kernel@i-love.sakura.ne.jp \
    --cc=andreyknvl@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hdanton@sina.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=syzbot+95ce4b142579611ef0a9@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=valentina.manea.m@gmail.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.