UbiquitousOS
/
OpenCloudOS-Kernel
mirror of https://gitee.com/OpenCloudOS/OpenCloudOS-Kernel.git
11
0
Fork 0
Code Issues Projects Releases Wiki Activity

HID: uhid: Fix worker destroying device without any protection 

uhid has to run hid_add_device() from workqueue context while allowing
parallel use of the userspace API (which is protected with ->devlock).
But hid_add_device() can fail. Currently, that is handled by immediately
destroying the associated HID device, without using ->devlock - but if
there are concurrent requests from userspace, that's wrong and leads to
NULL dereferences and/or memory corruption (via use-after-free).

Fix it by leaving the HID device as-is in the worker. We can clean it up
later, either in the UHID_DESTROY command handler or in the ->release()
handler.

Cc: stable@vger.kernel.org
Fixes: 67f8ecc550 ("HID: uhid: fix timeout when probe races with IO")
Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
This commit is contained in:
Jann Horn 2022-01-14 14:33:30 +01:00 committed by Jiri Kosina
parent e24aeff6db
commit 4ea5763fb7
1 changed files with 25 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
drivers/hid/uhid.c
View File

@ -28,11 +28,22 @@
struct uhid_device {
struct mutex devlock;
/* This flag tracks whether the HID device is usable for commands from
* userspace. The flag is already set before hid_add_device(), which
* runs in workqueue context, to allow hid_add_device() to communicate
* with userspace.
* However, if hid_add_device() fails, the flag is cleared without
* holding devlock.
* We guarantee that if @running changes from true to false while you're
* holding @devlock, it's still fine to access @hid.
*/
bool running;
__u8 *rd_data;
uint rd_size;
/* When this is NULL, userspace may use UHID_CREATE/UHID_CREATE2. */
struct hid_device *hid;
struct uhid_event input_buf;
@ -63,9 +74,18 @@ static void uhid_device_add_worker(struct work_struct *work)
if (ret) {
hid_err(uhid->hid, "Cannot register HID device: error %d\n", ret);
hid_destroy_device(uhid->hid);
uhid->hid = NULL;
/* We used to call hid_destroy_device() here, but that's really
* messy to get right because we have to coordinate with
* concurrent writes from userspace that might be in the middle
* of using uhid->hid.
* Just leave uhid->hid as-is for now, and clean it up when
* userspace tries to close or reinitialize the uhid instance.
*
* However, we do have to clear the ->running flag and do a
* wakeup to make sure userspace knows that the device is gone.
*/
uhid->running = false;
wake_up_interruptible(&uhid->report_wait);
}
}
@ -474,7 +494,7 @@ static int uhid_dev_create2(struct uhid_device *uhid,
void *rd_data;
int ret;
if (uhid->running)
if (uhid->hid)
return -EALREADY;
rd_size = ev->u.create2.rd_size;
@ -556,7 +576,7 @@ static int uhid_dev_create(struct uhid_device *uhid,
static int uhid_dev_destroy(struct uhid_device *uhid)
{
if (!uhid->running)
if (!uhid->hid)
return -EINVAL;
uhid->running = false;
@ -565,6 +585,7 @@ static int uhid_dev_destroy(struct uhid_device *uhid)
cancel_work_sync(&uhid->worker);
hid_destroy_device(uhid->hid);
uhid->hid = NULL;
kfree(uhid->rd_data);
return 0;