[PATCH 3/7] Input: gscps2 - protect buffer access in read and report helpers

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

In gscps2_report_data(), the ring buffer consumer index ps2port->act was
read and updated locklessly. When gscps2_interrupt() was called from
process context (such as during port write or open) concurrently with a
hardware interrupt running on another CPU, two execution contexts could
execute gscps2_report_data() simultaneously for the same port, racing on
ps2port->act and leading to duplicate, skipped, or out-of-order bytes.

Protect buffer access by taking ps2port->lock inside gscps2_read_data()
and gscps2_report_data(). In gscps2_report_data(), acquire ps2port->lock
only when popping entries from the ring buffer and release it before
calling serio_interrupt() to avoid recursive deadlocks if the input
driver synchronously sends a command back via serio_write().

Reported-by: sashiko-bot@kernel.org
Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/serio/gscps2.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
index 5b6e311f8a02..fef6fffb6f86 100644
--- a/drivers/input/serio/gscps2.c
+++ b/drivers/input/serio/gscps2.c
@@ -238,6 +238,8 @@ static void gscps2_read_data(struct gscps2port *ps2port)
 {
 	u8 status;
 
+	guard(spinlock_irqsave)(&ps2port->lock);
+
 	do {
 		status = gscps2_readb_status(ps2port->addr);
 		if (!(status & GSC_STAT_RBNE))
@@ -255,7 +257,7 @@ static bool gscps2_report_data(struct gscps2port *ps2port)
 	unsigned int rxflags;
 	u8 data, status;
 
-	while (ps2port->act != ps2port->append) {
+	while (true) {
 		/*
 		 * Did new data arrived while we read existing data ?
 		 * If yes, exit now and let the new irq handler start
@@ -264,17 +266,20 @@ static bool gscps2_report_data(struct gscps2port *ps2port)
 		if (gscps2_readb_status(ps2port->addr) & GSC_STAT_CMPINTR)
 			return true;
 
-		status = ps2port->buffer[ps2port->act].str;
-		data   = ps2port->buffer[ps2port->act].data;
+		scoped_guard(spinlock_irqsave, &ps2port->lock) {
+			if (ps2port->act == ps2port->append)
+				return false;
+
+			status = ps2port->buffer[ps2port->act].str;
+			data   = ps2port->buffer[ps2port->act].data;
+			ps2port->act = (ps2port->act + 1) & BUFFER_SIZE;
+		}
 
-		ps2port->act = (ps2port->act + 1) & BUFFER_SIZE;
 		rxflags = ((status & GSC_STAT_TERR) ? SERIO_TIMEOUT : 0) |
 			  ((status & GSC_STAT_PERR) ? SERIO_PARITY  : 0);
 
 		serio_interrupt(ps2port->port, data, rxflags);
 	}
-
-	return false;
 }
 
 /**
@@ -296,11 +301,8 @@ static irqreturn_t gscps2_interrupt(int irq, void *dev)
 
 	guard(rcu)();
 
-	list_for_each_entry_rcu(ps2port, &ps2port_list, node) {
-		guard(spinlock_irqsave)(&ps2port->lock);
-
+	list_for_each_entry_rcu(ps2port, &ps2port_list, node)
 		gscps2_read_data(ps2port);
-	}
 
 	/* all data was read from the ports - now report the data to upper layer */
 	list_for_each_entry_rcu(ps2port, &ps2port_list, node) {

-- 
2.55.0.897.gb25b4bd76c-goog