[PATCH] HID: roccat: fully initialize device before publishing to devices[]

[Ivy Lopez <skunkolee@gmail.com>]

roccat_connect() published the new device into the global devices[]
array, then released devices_lock, before initializing readers_lock,
cbuf_lock, the wait queue, the readers list, or setting hid/exist/
cbuf_end/report_size on the device.

Since devices[] is checked (under devices_lock) by roccat_open() to
decide whether a device is available to open, a concurrent open()
racing against roccat_connect() could look up the device, then lock
readers_lock before mutex_init() has run on it, or read device->hid
while it is still NULL from kzalloc, once devices_lock is released
but before the remaining fields are set.

Move all initialization of the device's own private state ahead of
the point where it is inserted into devices[], so the object is fully
constructed before it becomes visible to any other reader.

Reported-by: syzbot+9f7405999979761b6cfc@syzkaller.appspotmail.com
Signed-off-by: Ivy Lopez <skunkolee@gmail.com>
---
 drivers/hid/hid-roccat.c | 29 ++++++++++++-----------------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c
index d6fff53d4ee7..53358297e96c 100644
--- a/drivers/hid/hid-roccat.c
+++ b/drivers/hid/hid-roccat.c
@@ -307,26 +307,32 @@ int roccat_connect(const struct class *klass, struct hid_device *hid, int report
 	if (!device)
 		return -ENOMEM;
 
-	mutex_lock(&devices_lock);
+	init_waitqueue_head(&device->wait);
+	INIT_LIST_HEAD(&device->readers);
+	mutex_init(&device->readers_lock);
+	mutex_init(&device->cbuf_lock);
+	device->hid = hid;
+	device->exist = 1;
+	device->cbuf_end = 0;
+	device->report_size = report_size;
 
+	mutex_lock(&devices_lock);
 	for (minor = 0; minor < ROCCAT_MAX_DEVICES; ++minor) {
 		if (devices[minor])
 			continue;
 		break;
 	}
-
-	if (minor < ROCCAT_MAX_DEVICES) {
-		devices[minor] = device;
-	} else {
+	if (minor >= ROCCAT_MAX_DEVICES) {
 		mutex_unlock(&devices_lock);
 		kfree(device);
 		return -EINVAL;
 	}
+	device->minor = minor;
+	devices[minor] = device;
 
 	device->dev = device_create(klass, &hid->dev,
 			MKDEV(roccat_major, minor), NULL,
 			"%s%s%d", "roccat", hid->driver->name, minor);
-
 	if (IS_ERR(device->dev)) {
 		devices[minor] = NULL;
 		mutex_unlock(&devices_lock);
@@ -334,19 +340,8 @@ int roccat_connect(const struct class *klass, struct hid_device *hid, int report
 		kfree(device);
 		return temp;
 	}
-
 	mutex_unlock(&devices_lock);
 
-	init_waitqueue_head(&device->wait);
-	INIT_LIST_HEAD(&device->readers);
-	mutex_init(&device->readers_lock);
-	mutex_init(&device->cbuf_lock);
-	device->minor = minor;
-	device->hid = hid;
-	device->exist = 1;
-	device->cbuf_end = 0;
-	device->report_size = report_size;
-
 	return minor;
 }
 EXPORT_SYMBOL_GPL(roccat_connect);
-- 
2.55.0