[PATCH v2 02/11] HID: hid-lenovo-go-s: Bound stale reply window before reusing send_cmd_complete

[Derek J. Clark]

mcu_property_out() reinits send_cmd_complete immediately after a
timeout is detected, before the next command that reuses it is sent.
If the MCU's reply to the timed-out command arrives after this reinit
but before the next command's wait begins, it silently satisfies the
next, unrelated command's wait instead of the one it actually answers,
handing that caller stale data with no way to detect the mismatch.

Track when no reply was consumed, i.e. the wait timed out or was
interrupted, via orphan_ack_pending. Before the next command reuses
the completion, wait a bounded 25ms for a stale reply to arrive and be
consumed, then unconditionally clear the flag and reinit the
completion. This does not fully eliminate the window in which an
unrelated reply could still be received, but bounds it to a short
interval right before a new command is sent. Behavior matches the
solution to the same problem in hid-msi.

Fixes: a23f3497bf208c59ad ("HID: hid-lenovo-go-s: Add Lenovo Legion Go S Series HID Driver")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Derek J. Clark <derekjohn.clark@xxxxxxxxx>
---
 drivers/hid/hid-lenovo-go-s.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-lenovo-go-s.c b/drivers/hid/hid-lenovo-go-s.c
index 68301d4c762a..e979527ece8e 100644
--- a/drivers/hid/hid-lenovo-go-s.c
+++ b/drivers/hid/hid-lenovo-go-s.c
@@ -37,6 +37,7 @@ static struct hid_gos_cfg {
 	struct completion send_cmd_complete;
 	struct led_classdev *led_cdev;
 	struct hid_device *hdev;
+	bool orphan_ack_pending;
 	struct mutex cfg_mutex; /*ensure single synchronous output report*/
 	int cmd_status;
 	u8 gp_auto_sleep_time;
@@ -448,12 +449,26 @@ static int mcu_property_out(struct hid_device *hdev, u8 command, u8 index,
 	unsigned char *dmabuf __free(kfree) = NULL;
 	u8 header[] = { command, index };
 	size_t header_size = ARRAY_SIZE(header);
+	long t_remaining;
 	int timeout, ret;
 
 	if (header_size + len > GO_S_PACKET_SIZE)
 		return -EINVAL;
 
 	guard(mutex)(&drvdata.cfg_mutex);
+
+	/*
+	 * A reply to the previous command may still be in flight. Give it a
+	 * short window to arrive and be consumed before this call reinits the
+	 * completion, so a late reply can't be mistaken for this command's.
+	 */
+	if (drvdata.orphan_ack_pending) {
+		wait_for_completion_timeout(&drvdata.send_cmd_complete, msecs_to_jiffies(25));
+		drvdata.orphan_ack_pending = false;
+		drvdata.cmd_status = -ETIMEDOUT;
+	}
+	reinit_completion(&drvdata.send_cmd_complete);
+
 	/* We can't use a devm_alloc reusable buffer without side effects during suspend */
 	dmabuf = kzalloc(GO_S_PACKET_SIZE, GFP_KERNEL);
 	if (!dmabuf)
@@ -475,11 +490,15 @@ static int mcu_property_out(struct hid_device *hdev, u8 command, u8 index,
 
 	/* PL_TEST commands can take longer because they go out to another device */
 	timeout = (command == GET_PL_TEST) ? 200 : 5;
-	ret = wait_for_completion_interruptible_timeout(&drvdata.send_cmd_complete,
-							msecs_to_jiffies(timeout));
-	ret = ret > 0 ? drvdata.cmd_status : ret ?: -EBUSY;
+	t_remaining = wait_for_completion_interruptible_timeout(&drvdata.send_cmd_complete,
+								msecs_to_jiffies(timeout));
+	if (t_remaining <= 0) {
+		/* A reply to this command may still be in flight. */
+		drvdata.orphan_ack_pending = true;
+	}
+
+	ret = t_remaining > 0 ? drvdata.cmd_status : t_remaining ?: -EBUSY;
 
-	reinit_completion(&drvdata.send_cmd_complete);
 	return ret;
 }
 
-- 
2.55.0