[PATCH 1/4] HID: cougar: fix out-of-bounds read in cougar_raw_event

[PATCH 1/4] HID: cougar: fix out-of-bounds read in cougar_raw_event

[Jiale Yao]

cougar_raw_event() accesses data[COUGAR_FIELD_CODE] and
data[COUGAR_FIELD_ACTION] (offsets 1 and 2) before validating
the report size.  A malformed HID report shorter than 3 bytes
would read past the allocated buffer.

Add a size < 3 check before accessing those fields.

Assisted-by: Claude:deepseek-v4-pro
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/hid/hid-cougar.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/hid/hid-cougar.c b/drivers/hid/hid-cougar.c
index ad027c45f162..9927f45a795b 100644
--- a/drivers/hid/hid-cougar.c
+++ b/drivers/hid/hid-cougar.c
@@ -270,6 +270,12 @@ static int cougar_raw_event(struct hid_device *hdev, struct hid_report *report,
 	if (!shared->enabled || !shared->input)
 		return -EPERM;
 
+	if (size < 3) {
+		hid_err(hdev, "Received HID report of bad size (%d)",
+			size);
+		return -EPERM;
+	}
+
 	code = data[COUGAR_FIELD_CODE];
 	action = data[COUGAR_FIELD_ACTION];
 	for (i = 0; cougar_mapping[i][0]; i++) {
-- 
2.34.1

[PATCH 2/4] HID: corsair-void: fix out-of-bounds read in corsair_void_raw_event

[Jiale Yao]

Both the STATUS_REPORT and FIRMWARE_REPORT paths access data[4].
Add a size < 5 guard before the report dispatch.

Assisted-by: Claude:deepseek-v4-pro
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/hid/hid-corsair-void.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hid/hid-corsair-void.c b/drivers/hid/hid-corsair-void.c
index 5e9a5b8f7f16..7c5435de6aae 100644
--- a/drivers/hid/hid-corsair-void.c
+++ b/drivers/hid/hid-corsair-void.c
@@ -740,6 +740,8 @@ static int corsair_void_raw_event(struct hid_device *hid_dev,
 	struct corsair_void_drvdata *drvdata = hid_get_drvdata(hid_dev);
 	bool was_connected = drvdata->connected;
 
+	if (size < 5)
+		return 0;
 	/* Description of packets are documented at the top of this file */
 	if (hid_report->id == CORSAIR_VOID_STATUS_REPORT_ID) {
 		drvdata->mic_up = FIELD_GET(CORSAIR_VOID_MIC_MASK, data[2]);
-- 
2.34.1

[PATCH 3/4] HID: mcp2221: fix out-of-bounds read in mcp2221_raw_event

[Jiale Yao]

The handler dispatches on data[0] without checking size >= 1.
Add a top-level guard and per-case minimum-size checks:
data[2] for the I2C_WR_DATA group, data[20] and
data[50+sizeof(adc_values)] for I2C_PARAM_OR_STATUS,
sizeof(struct mcp_get/set_gpio) for GPIO paths,
and 4+data[3] for the I2C_GET_DATA partial-read memcpy.

Assisted-by: Claude:deepseek-v4-pro
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/hid/hid-mcp2221.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
index e4ddd8e9293b..0423b5affe8f 100644
--- a/drivers/hid/hid-mcp2221.c
+++ b/drivers/hid/hid-mcp2221.c
@@ -861,12 +861,17 @@ static int mcp2221_raw_event(struct hid_device *hdev,
 	u8 *buf;
 	struct mcp2221 *mcp = hid_get_drvdata(hdev);
 
+	if (size < 1)
+		return 1;
+
 	switch (data[0]) {
 
 	case MCP2221_I2C_WR_DATA:
 	case MCP2221_I2C_WR_NO_STOP:
 	case MCP2221_I2C_RD_DATA:
 	case MCP2221_I2C_RD_RPT_START:
+		if (size < 3)
+			return 1;
 		switch (data[1]) {
 		case MCP2221_SUCCESS:
 			mcp->status = 0;
@@ -878,6 +883,12 @@ static int mcp2221_raw_event(struct hid_device *hdev,
 		break;
 
 	case MCP2221_I2C_PARAM_OR_STATUS:
+		if (size < 21)
+			return 1;
+#if IS_REACHABLE(CONFIG_IIO)
+		if (size < 50 + sizeof(mcp->adc_values))
+			return 1;
+#endif
 		switch (data[1]) {
 		case MCP2221_SUCCESS:
 			if ((mcp->txbuf[3] == MCP2221_I2C_SET_SPEED) &&
@@ -901,6 +912,8 @@ static int mcp2221_raw_event(struct hid_device *hdev,
 		break;
 
 	case MCP2221_I2C_GET_DATA:
+		if (size < 4)
+			return 1;
 		switch (data[1]) {
 		case MCP2221_SUCCESS:
 			if (data[2] == MCP2221_I2C_ADDR_NACK) {
@@ -918,7 +931,8 @@ static int mcp2221_raw_event(struct hid_device *hdev,
 			}
 			if (data[2] == MCP2221_I2C_READ_COMPL ||
 			    data[2] == MCP2221_I2C_READ_PARTIAL) {
-				if (!mcp->rxbuf || mcp->rxbuf_idx < 0 || data[3] > 60) {
+				if (!mcp->rxbuf || mcp->rxbuf_idx < 0 ||
+				    data[3] > 60 || size < 4 + data[3]) {
 					mcp->status = -EINVAL;
 					break;
 				}
@@ -941,6 +955,8 @@ static int mcp2221_raw_event(struct hid_device *hdev,
 		break;
 
 	case MCP2221_GPIO_GET:
+		if (size < sizeof(struct mcp_get_gpio))
+			return 1;
 		switch (data[1]) {
 		case MCP2221_SUCCESS:
 			if ((data[mcp->gp_idx] == MCP2221_ALT_F_NOT_GPIOV) ||
@@ -958,6 +974,8 @@ static int mcp2221_raw_event(struct hid_device *hdev,
 		break;
 
 	case MCP2221_GPIO_SET:
+		if (size < sizeof(struct mcp_set_gpio))
+			return 1;
 		switch (data[1]) {
 		case MCP2221_SUCCESS:
 			if ((data[mcp->gp_idx] == MCP2221_ALT_F_NOT_GPIOV) ||
@@ -974,6 +992,8 @@ static int mcp2221_raw_event(struct hid_device *hdev,
 		break;
 
 	case MCP2221_SET_SRAM_SETTINGS:
+		if (size < 2)
+			return 1;
 		switch (data[1]) {
 		case MCP2221_SUCCESS:
 			mcp->status = 0;
@@ -985,6 +1005,8 @@ static int mcp2221_raw_event(struct hid_device *hdev,
 		break;
 
 	case MCP2221_GET_SRAM_SETTINGS:
+		if (size < 26)
+			return 1;
 		switch (data[1]) {
 		case MCP2221_SUCCESS:
 			memcpy(&mcp->mode, &data[22], 4);
@@ -1000,6 +1022,8 @@ static int mcp2221_raw_event(struct hid_device *hdev,
 		break;
 
 	case MCP2221_READ_FLASH_DATA:
+		if (size < 8)
+			return 1;
 		switch (data[1]) {
 		case MCP2221_SUCCESS:
 			mcp->status = 0;
-- 
2.34.1

Re: [PATCH 3/4] HID: mcp2221: fix out-of-bounds read in mcp2221_raw_event

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Stale mcp->rxbuf pointer leads to Use-After-Free or Stack Buffer Overflow when receiving delayed MCP2221_I2C_GET_DATA input reports.
- [High] An out-of-bounds read occurs in mcp2221_raw_event due to uninitialized or shared mcp->gp_idx state.
- [High] Shared transfer state variables such as mcp->rxbuf_idx are modified concurrently by process context and the HID raw event handler without locking.
- [High] The `retries` counter in `mcp_init_work` is declared as a `static int`, leading to data races and logic bugs when multiple mcp2221 devices are probed or a single device is replugged.
--

--- Patch [3]: [PATCH 3/4] HID: mcp2221: fix out-of-bounds read in mcp2221_raw_event ---
commit 8bc8521d47c1c74b70fcc5afb281169b37669d79
Author: Jiale Yao <yaojiale02@163.com>

HID: mcp2221: fix out-of-bounds read in mcp2221_raw_event

This commit adds bounds checking to the mcp2221_raw_event handler to
prevent out-of-bounds reads when processing HID reports. It introduces a
top-level size check and specific size constraints for different I2C and
GPIO operations.

> diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
> index e4ddd8e9293b6..0423b5affe8f6 100644
> --- a/drivers/hid/hid-mcp2221.c
> +++ b/drivers/hid/hid-mcp2221.c
[ ... ]
> @@ -918,7 +931,8 @@ static int mcp2221_raw_event(struct hid_device *hdev,
>  			}
>  			if (data[2] == MCP2221_I2C_READ_COMPL ||
>  			    data[2] == MCP2221_I2C_READ_PARTIAL) {
> -				if (!mcp->rxbuf || mcp->rxbuf_idx < 0 || data[3] > 60) {
> +				if (!mcp->rxbuf || mcp->rxbuf_idx < 0 ||
> +				    data[3] > 60 || size < 4 + data[3]) {
>  					mcp->status = -EINVAL;
>  					break;
>  				}

[Severity: Critical]
This is a pre-existing issue, but does this code write to a dangling
stack pointer if a delayed input report arrives?

Looking at mcp_i2c_smbus_read(), mcp->rxbuf is pointed to a local stack
buffer. If the I2C request times out after 5 retries, mcp_i2c_smbus_read()
returns without clearing mcp->rxbuf:

mcp_i2c_smbus_read() {
    ...
    retries++;
    if (retries == 5)
        return -ETIMEDOUT;
    ...
}

Could a malicious USB device send a delayed MCP2221_I2C_GET_DATA report
after the timeout, causing mcp2221_raw_event() to locklessly copy up to
60 bytes into the now-freed stack frame?

[Severity: High]
This is also a pre-existing issue, but is it safe to modify mcp->rxbuf_idx
here without a lock?

The I2C process thread holds mcp->lock while initializing mcp->rxbuf_idx
in mcp_i2c_smbus_read():

mcp_i2c_smbus_read() {
    ...
    mcp->rxbuf_idx = 0;
    ...
}

However, mcp2221_raw_event() runs asynchronously in softirq context. Can a
spurious input report race with the process-context operations, corrupting
the buffer bounds or indices?

[ ... ]
> @@ -958,6 +974,8 @@ static int mcp2221_raw_event(struct hid_device *hdev,
>  		break;
>  
>  	case MCP2221_GPIO_SET:
> +		if (size < sizeof(struct mcp_set_gpio))
> +			return 1;
>  		switch (data[1]) {
>  		case MCP2221_SUCCESS:
>  			if ((data[mcp->gp_idx] == MCP2221_ALT_F_NOT_GPIOV) ||

[Severity: High]
This is a pre-existing issue, but does this code perform an out-of-bounds
access if a device sends an unsolicited report before any GPIO operations
occur?

If mcp->gp_idx is still 0, evaluating data[mcp->gp_idx - 1] in the
MCP2221_GPIO_SET handler will read from data[-1].

Similarly, if mcp->gp_idx is legitimately set to a high value like 17,
could an unsolicited MCP2221_GPIO_GET report with a valid size of 10
access data[18], reading past the bounds of the short packet? The newly
added check for the overall packet size does not prevent this since it
doesn't validate the independently modified mcp->gp_idx.

[ ... ]

[Severity: High]
This is a pre-existing issue outside the modified hunks, but is the
retries counter in mcp_init_work() susceptible to data races across
multiple devices?

mcp_init_work() defines retries as a static variable:

mcp_init_work() {
    static int retries = 5;
    ...
    if (!retries--)
        return;
    ...
    schedule_delayed_work(&mcp->init_work, msecs_to_jiffies(100));
}

If a device fails IIO initialization multiple times, the static counter
drops to -1. Once negative, !retries-- evaluates to false, causing an
infinite rescheduling loop for any subsequent devices or replugs that
fail. Should this retry state be tracked per-device in the mcp2221
struct instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714131611.269929-1-yaojiale02@163.com?part=3

[PATCH 4/4] HID: cp2112: fix out-of-bounds read in cp2112_raw_event

[Jiale Yao]

The TRANSFER_STATUS_RESPONSE path dereferences xfer->length at
offset 6, requiring sizeof(*xfer) bytes.  The DATA_READ_RESPONSE
path uses the device-supplied read_length to drive memcpy()
without clamping it to the remaining report size.  Add both
checks, and clamp read_length to size - 3.

Assisted-by: Claude:deepseek-v4-pro
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/hid/hid-cp2112.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 04379db93571..99e31724a421 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -1430,6 +1430,11 @@ static int cp2112_raw_event(struct hid_device *hdev, struct hid_report *report,
 
 	switch (data[0]) {
 	case CP2112_TRANSFER_STATUS_RESPONSE:
+		if (size < sizeof(*xfer)) {
+			hid_err(hdev, "short transfer status report(%d < %zu)\n", size,
+				sizeof(*xfer));
+			return 0;
+		}
 		hid_dbg(hdev, "xfer status: %02x %02x %04x %04x\n",
 			xfer->status0, xfer->status1,
 			be16_to_cpu(xfer->retries), be16_to_cpu(xfer->length));
@@ -1463,12 +1468,18 @@ static int cp2112_raw_event(struct hid_device *hdev, struct hid_report *report,
 		atomic_set(&dev->xfer_avail, 1);
 		break;
 	case CP2112_DATA_READ_RESPONSE:
+		if (size < 4) {
+			hid_err(hdev, "short data read response(%d < 4)\n", size);
+			return 0;
+		}
 		hid_dbg(hdev, "read response: %02x %02x\n", data[1], data[2]);
 
 		dev->read_length = data[2];
 		if (dev->read_length > sizeof(dev->read_data))
 			dev->read_length = sizeof(dev->read_data);
 
+		if (dev->read_length > size - 3)
+			dev->read_length = size - 3;
 		memcpy(dev->read_data, &data[3], dev->read_length);
 		atomic_set(&dev->read_avail, 1);
 		break;
-- 
2.34.1

Re: [PATCH 4/4] HID: cp2112: fix out-of-bounds read in cp2112_raw_event

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing cancel_delayed_work_sync() on the probe error path can lead to Use-After-Free of devm-allocated dev structure.
- [High] Race condition in cp2112_remove() due to premature and conditional cancellation of the delayed work.
- [High] Lockless updates to dev->read_data and dev->read_length lack required memory barriers when signaled via atomic flags.
- [High] A TOCTOU race condition on dev->read_length in cp2112_read() can bypass length checks and return uninitialized kernel memory.
- [High] Using an interruptible wait in cp2112_wait() allows signals to permanently desynchronize the hardware response state.
--

--- Patch [4]: [PATCH 4/4] HID: cp2112: fix out-of-bounds read in cp2112_raw_event ---
commit 1ea70eb400858ee9a4d1aa2528079b6370e4b0a8
Author: Jiale Yao <yaojiale02@163.com>

HID: cp2112: fix out-of-bounds read in cp2112_raw_event

The TRANSFER_STATUS_RESPONSE path dereferences xfer->length at
offset 6, requiring sizeof(*xfer) bytes.  The DATA_READ_RESPONSE
path uses the device-supplied read_length to drive memcpy()
without clamping it to the remaining report size.  Add both
checks, and clamp read_length to size - 3.

> diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
> index 04379db93571b..99e31724a4211 100644
> --- a/drivers/hid/hid-cp2112.c
> +++ b/drivers/hid/hid-cp2112.c

[ ... ]

> @@ -1463,12 +1468,18 @@ static int cp2112_raw_event(struct hid_device *hdev, struct hid_report *report,
>  		atomic_set(&dev->xfer_avail, 1);
>  		break;
>  	case CP2112_DATA_READ_RESPONSE:
> +		if (size < 4) {
> +			hid_err(hdev, "short data read response(%d < 4)\n", size);
> +			return 0;
> +		}
>  		hid_dbg(hdev, "read response: %02x %02x\n", data[1], data[2]);
>  
>  		dev->read_length = data[2];
>  		if (dev->read_length > sizeof(dev->read_data))
>  			dev->read_length = sizeof(dev->read_data);
>  
> +		if (dev->read_length > size - 3)
> +			dev->read_length = size - 3;
>  		memcpy(dev->read_data, &data[3], dev->read_length);
>  		atomic_set(&dev->read_avail, 1);

[Severity: High]
This is a pre-existing issue, but does this lockless update to dev->read_data
and dev->read_length lack required memory barriers?

In cp2112_raw_event(), dev->read_data is updated and then read_avail is
signaled locklessly:

    memcpy(dev->read_data, &data[3], dev->read_length);
    atomic_set(&dev->read_avail, 1);

However, the consumer in cp2112_wait() evaluates atomic_read() locklessly,
and cp2112_read() reads the payload without an acquire barrier:

cp2112_wait():
    ret = wait_event_interruptible_timeout(dev->wait,
        atomic_read(avail), msecs_to_jiffies(RESPONSE_TIMEOUT));

cp2112_read():
    memcpy(data, dev->read_data, size);

On weakly ordered architectures, could the CPU reorder the producer's data
write after the flag set, or the consumer's data read before the flag check,
leading to stale memory reads and I2C data corruption?


[Severity: High]
This is a pre-existing issue, but does the error path in cp2112_probe()
leave the delayed work running?

drivers/hid/hid-cp2112.c:cp2112_probe() {
...
err_gpiochip_remove:
    gpiochip_remove(&dev->gc);
err_free_i2c:
...
}

If gpiochip_add_data() succeeds, the GPIO chip is exposed and its IRQ can be
immediately requested, scheduling the gpio_poll_worker.

If a subsequent initialization step fails, the probe function jumps to
err_gpiochip_remove and removes the gpiochip, but fails to cancel the
delayed work before the dev structure is freed.

Could this lead to a Use-After-Free of the dev structure?


[Severity: High]
This is a pre-existing issue, but is there a race condition in
cp2112_remove() due to premature cancellation of the delayed work?

drivers/hid/hid-cp2112.c:cp2112_remove() {
...
    if (dev->gpio_poll) {
        dev->gpio_poll = false;
        cancel_delayed_work_sync(&dev->gpio_poll_worker);
    }

    gpiochip_remove(&dev->gc);
...
}

Because the GPIO chip is still registered when the cancellation is evaluated,
a concurrent IRQ request can execute cp2112_gpio_irq_startup(), setting
dev->gpio_poll = true and scheduling the work just after the cancellation
check but before the chip is removed.

Once remove() completes and dev is freed, will the scheduled work cause a
Use-After-Free?


[Severity: High]
This is a pre-existing issue, but is there a TOCTOU race on
dev->read_length in cp2112_read()?

drivers/hid/hid-cp2112.c:cp2112_read() {
...
    if (size > dev->read_length)
        size = dev->read_length;

    memcpy(data, dev->read_data, size);
    return dev->read_length;
}

If a spurious or delayed USB CP2112_DATA_READ_RESPONSE packet is processed
concurrently by cp2112_raw_event(), dev->read_length can increase between
the bounds check and the return value.

Since cp2112_read() reads dev->read_length locklessly twice, could it copy
fewer bytes than it returns? This would cause the caller cp2112_i2c_xfer()
to advance its buffer by the larger returned value and leak uninitialized
heap memory to the userspace I2C client.


[Severity: High]
This is a pre-existing issue, but does using an interruptible wait in
cp2112_wait() allow signals to permanently desynchronize the hardware
response state?

drivers/hid/hid-cp2112.c:cp2112_wait() {
...
    ret = wait_event_interruptible_timeout(dev->wait,
        atomic_read(avail), msecs_to_jiffies(RESPONSE_TIMEOUT));
    if (-ERESTARTSYS == ret)
        return ret;
...
}

If a user process executing an I2C transaction receives a signal,
wait_event_interruptible_timeout() aborts with -ERESTARTSYS. The driver
exits, but the hardware will eventually deliver the response for the
aborted transaction.

Because the hardware lacks transaction IDs, will this late response falsely
satisfy the cp2112_wait() of the next I2C transaction, returning garbage
data or false success?

Should the driver use wait_event_timeout() for short hardware
synchronization instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714131611.269929-1-yaojiale02@163.com?part=4

Re: [PATCH 1/4] HID: cougar: fix out-of-bounds read in cougar_raw_event

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:

New issues:
- [Medium] Unbounded error logging in interrupt context can lead to log flooding and Denial of Service.
- [Low] The newly added `hid_err` log message is missing a terminating newline character.

Pre-existing issues:
- [High] TOCTOU race condition during device unbinding leads to a Use-After-Free of the keyboard's input_dev (`shared->input`).
- [High] Self-deadlock on `cougar_udev_list_lock` during error handling in `cougar_bind_shared_data()`.
- [High] Race condition between `kref_put()` and list lookup causing Use-After-Free of the `cougar_shared` structure.
- [High] The probe function dereferences `hdev->collection` without verifying that any collections were actually parsed.
--

--- Patch [1]: [PATCH 1/4] HID: cougar: fix out-of-bounds read in cougar_raw_event ---
commit bb404070c5edd2b402b1e549ba66068e04c7d429
Author: Jiale Yao <yaojiale02@163.com>

HID: cougar: fix out-of-bounds read in cougar_raw_event

This patch adds a bounds check to cougar_raw_event() to prevent out-of-bounds
reads when handling malformed HID reports shorter than 3 bytes.

> --- a/drivers/hid/hid-cougar.c
> +++ b/drivers/hid/hid-cougar.c
> @@ -270,6 +270,12 @@ static int cougar_raw_event(struct hid_device *hdev, struct hid_report *report,
>  	if (!shared->enabled || !shared->input)
>  		return -EPERM;

[Severity: High]
This is a pre-existing issue, but is there a TOCTOU race condition here
during device unbinding?

If a device disconnect is triggered while a USB event is being processed,
cougar_raw_event() checks shared->enabled locklessly on one CPU.
Concurrently, cougar_remove() on another CPU could clear shared->enabled
and call hid_hw_stop(), destroying the input_dev.

Could the interrupt handler then resume and dereference the freed
shared->input in input_event()?

> +	if (size < 3) {
> +		hid_err(hdev, "Received HID report of bad size (%d)",
> +			size);

[Severity: Medium]
Could this un-ratelimited error logging lead to a log flood if a
malfunctioning or malicious device continuously sends reports shorter than
3 bytes?

Since cougar_raw_event() runs in interrupt context, this might cause high
CPU usage and flood the kernel ring buffer.

[Severity: Low]
Also, does this log message need a terminating newline character?
Without the \n, subsequent kernel messages might be incorrectly appended
to the same line.

> +		return -EPERM;
> +	}
> +
>  	code = data[COUGAR_FIELD_CODE];

[ ... ]

[Severity: High]
This is a pre-existing issue, but does cougar_bind_shared_data() have a
self-deadlock on cougar_udev_list_lock during error handling?

If devm_add_action_or_reset() fails, it invokes cougar_remove_shared_data()
which drops the kref to zero, triggering cougar_release_shared_data():

drivers/hid/hid-cougar.c:cougar_release_shared_data() {
    ...
    mutex_lock(&cougar_udev_list_lock);
    ...
}

Since cougar_bind_shared_data() already holds cougar_udev_list_lock when
calling devm_add_action_or_reset(), will this attempt to re-acquire the
already held mutex block all further probes?

[Severity: High]
This is a pre-existing issue, but is there a race condition between
kref_put() and list lookup that could cause a use-after-free of the
cougar_shared structure?

During device unbind, cougar_remove_shared_data() calls
kref_put(&shared->kref) without holding cougar_udev_list_lock. If the
refcount drops to 0, cougar_release_shared_data() blocks on the lock.

Concurrently, a new device probe calling cougar_bind_shared_data() holds
the lock and traverses the list:

drivers/hid/hid-cougar.c:cougar_bind_shared_data() {
    ...
    mutex_lock(&cougar_udev_list_lock);
    list_for_each_entry(shared, &cougar_udev_list, list) {
        ...
        kref_get(&shared->kref);
        ...
    }
}

Could it find the zero-refcount object, increment it, and then after the lock
is dropped, the blocked release function frees it, leaving the newly
probed device with a dangling pointer?

[Severity: High]
This is a pre-existing issue, but does the probe function dereference
hdev->collection without verifying that any collections were actually
parsed?

drivers/hid/hid-cougar.c:cougar_probe() {
    ...
    if (hdev->collection->usage == COUGAR_VENDOR_USAGE) {
    ...
}

If a USB device supplies a HID report descriptor containing no collections,
hid_parse() succeeds but hdev->collection remains NULL. Will this
unconditional access cause a panic on probe?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714131611.269929-1-yaojiale02@163.com?part=1