linux-kernel - [PATCH 3.13 01/46] HID: hidraw: fix warning destroying hidraw device files after parent

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20140328173134.831298277@linuxfoundation.org>
Date:	Fri, 28 Mar 2014 10:31:45 -0700
From:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:	linux-kernel@...r.kernel.org
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	stable@...r.kernel.org,
	Fernando Luis Vazquez Cao <fernando@....ntt.co.jp>,
	David Herrmann <dh.herrmann@...il.com>,
	Jiri Kosina <jkosina@...e.cz>
Subject: [PATCH 3.13 01/46] HID: hidraw: fix warning destroying hidraw device files after parent

3.13-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Fernando Luis Vazquez Cao <fernando@....ntt.co.jp>

commit 47587fc098451c2100dc1fb618855fc2e2d937af upstream.

I noticed that after hot unplugging a Logitech unifying receiver
(drivers/hid/hid-logitech-dj.c) the kernel would occasionally spew a
stack trace similar to this:

usb 1-1.1.2: USB disconnect, device number 7
WARNING: CPU: 0 PID: 2865 at fs/sysfs/group.c:216 device_del+0x40/0x1b0()
sysfs group ffffffff8187fa20 not found for kobject 'hidraw0'
[...]
CPU: 0 PID: 2865 Comm: upowerd Tainted: G        W 3.14.0-rc4 #7
Hardware name: LENOVO 7783PN4/        , BIOS 9HKT43AUS 07/11/2011
 0000000000000009 ffffffff814cd684 ffff880427ccfdf8 ffffffff810616e7
 ffff88041ec61800 ffff880427ccfe48 ffff88041e444d80 ffff880426fab8e8
 ffff880429359960 ffffffff8106174c ffffffff81714b98 0000000000000028
Call Trace:
 [<ffffffff814cd684>] ? dump_stack+0x41/0x51
 [<ffffffff810616e7>] ? warn_slowpath_common+0x77/0x90
 [<ffffffff8106174c>] ? warn_slowpath_fmt+0x4c/0x50
 [<ffffffff81374fd0>] ? device_del+0x40/0x1b0
 [<ffffffff8137516f>] ? device_unregister+0x2f/0x50
 [<ffffffff813751fa>] ? device_destroy+0x3a/0x40
 [<ffffffffa03ca245>] ? drop_ref+0x55/0x120 [hid]
 [<ffffffffa03ca3e6>] ? hidraw_release+0x96/0xb0 [hid]
 [<ffffffff811929da>] ? __fput+0xca/0x210
 [<ffffffff8107fe17>] ? task_work_run+0x97/0xd0
 [<ffffffff810139a9>] ? do_notify_resume+0x69/0xa0
 [<ffffffff814dbd22>] ? int_signal+0x12/0x17
---[ end trace 63f4a46f6566d737 ]---

During device removal hid_disconnect() is called via hid_hw_stop() to
stop the device and free all its resources, including the sysfs
files. The problem is that if a user space process, such as upowerd,
holds a reference to a hidraw file the corresponding sysfs files will
be kept around (drop_ref() does not call device_destroy() if the open
counter is not 0) and it will be usb_disconnect() who, by calling
device_del() for the USB device, will indirectly remove the sysfs
files of the hidraw device (sysfs_remove_dir() is recursive these
days). Because of this, by the time user space releases the last
reference to the hidraw file and drop_ref() tries to destroy the
device the sysfs files are already gone and the kernel will print
the warning above.

Fix this by calling device_destroy() at USB disconnect time.

Signed-off-by: Fernando Luis Vazquez Cao <fernando@....ntt.co.jp>
Reviewed-by: David Herrmann <dh.herrmann@...il.com>
Signed-off-by: Jiri Kosina <jkosina@...e.cz>
Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>

---
 drivers/hid/hidraw.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/drivers/hid/hidraw.c
+++ b/drivers/hid/hidraw.c
@@ -313,13 +313,13 @@ static void drop_ref(struct hidraw *hidr
 			hid_hw_close(hidraw->hid);
 			wake_up_interruptible(&hidraw->wait);
 		}
+		device_destroy(hidraw_class,
+			       MKDEV(hidraw_major, hidraw->minor));
 	} else {
 		--hidraw->open;
 	}
 	if (!hidraw->open) {
 		if (!hidraw->exist) {
-			device_destroy(hidraw_class,
-					MKDEV(hidraw_major, hidraw->minor));
 			hidraw_table[hidraw->minor] = NULL;
 			kfree(hidraw);
 		} else {


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/