On Fri Jun 6 03:04:59 2025 +0000, Gui-Dong Han wrote:
> In the interrupt handler rain_interrupt(), the buffer full check on
> rain->buf_len is performed before acquiring rain->buf_lock. This
> creates a Time-of-Check to Time-of-Use (TOCTOU) race condition, as
> rain->buf_len is concurrently accessed and modified in the work
> handler rain_irq_work_handler() under the same lock.
>
> Multiple interrupt invocations can race, with each reading buf_len
> before it becomes full and then proceeding. This can lead to both
> interrupts attempting to write to the buffer, incrementing buf_len
> beyond its capacity (DATA_SIZE) and causing a buffer overflow.
>
> Fix this bug by moving the spin_lock() to before the buffer full
> check. This ensures that the check and the subsequent buffer modification
> are performed atomically, preventing the race condition. An corresponding
> spin_unlock() is added to the overflow path to correctly release the
> lock.
>
> This possible bug was found by an experimental static analysis tool
> developed by our team.
>
> Fixes: 0f314f6c2e77 ("[media] rainshadow-cec: new RainShadow Tech HDMI CEC
> driver")
> Cc: [email protected]
> Signed-off-by: Gui-Dong Han <[email protected]>
> Signed-off-by: Hans Verkuil <[email protected]>
Patch committed.
Thanks,
Hans Verkuil
drivers/media/cec/usb/rainshadow/rainshadow-cec.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
---
diff --git a/drivers/media/cec/usb/rainshadow/rainshadow-cec.c
b/drivers/media/cec/usb/rainshadow/rainshadow-cec.c
index ee870ea1a886..6f8d6797c614 100644
--- a/drivers/media/cec/usb/rainshadow/rainshadow-cec.c
+++ b/drivers/media/cec/usb/rainshadow/rainshadow-cec.c
@@ -171,11 +171,12 @@ static irqreturn_t rain_interrupt(struct serio *serio,
unsigned char data,
{
struct rain *rain = serio_get_drvdata(serio);
+ spin_lock(&rain->buf_lock);
if (rain->buf_len == DATA_SIZE) {
+ spin_unlock(&rain->buf_lock);
dev_warn_once(rain->dev, "buffer overflow\n");
return IRQ_HANDLED;
}
- spin_lock(&rain->buf_lock);
rain->buf_len++;
rain->buf[rain->buf_wr_idx] = data;
rain->buf_wr_idx = (rain->buf_wr_idx + 1) & 0xff;
