From: sashiko-bot@kernel.org
To: "Derek J. Clark" <derekjohn.clark@gmail.com>
Cc: dmitry.torokhov@gmail.com, linux-input@vger.kernel.org
Subject: Re: [PATCH 02/10] HID: hid-lenovo-go-s: Bound stale reply window before reusing send_cmd_complete
Date: Mon, 14 Sep 2026 23:26:55 +0000 [thread overview]
Message-ID: <20260914232656.45A751F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260914225303.868569-3-derekjohn.clark@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] hid: conflating MCU error with timeout in mcu_property_out()
--
commit b76b36bd4347a4e16200e7b5eadeaf196d8595bd
Author: Derek J. Clark <derekjohn.clark@gmail.com>
Subject: HID: hid-lenovo-go-s: Bound stale reply window before reusing send_cmd_complete
This patch mitigates an issue where stale replies to timed-out commands can
silently satisfy the wait for the next command. It introduces a short bounded
wait before reusing the completion structure and tracks when a command has
timed out using a new pending flag.
> diff --git a/drivers/hid/hid-lenovo-go-s.c b/drivers/hid/hid-lenovo-go-s.c
> index 68301d4c762a1..36505d8402ff1 100644
> --- a/drivers/hid/hid-lenovo-go-s.c
> +++ b/drivers/hid/hid-lenovo-go-s.c
> @@ -479,7 +493,9 @@ static int mcu_property_out(struct hid_device *hdev, u8 command, u8 index,
> msecs_to_jiffies(timeout));
> ret = ret > 0 ? drvdata.cmd_status : ret ?: -EBUSY;
>
> - reinit_completion(&drvdata.send_cmd_complete);
> + if (ret)
> + drvdata.orphan_ack_pending = true;
[Severity: Low]
Could this logic inadvertently flag a successfully received error reply as an
orphaned timeout?
When the MCU successfully replies with an error code (meaning drvdata.cmd_status
is negative), wait_for_completion_interruptible_timeout() returns a positive
value indicating the wait succeeded. The code then assigns ret =
drvdata.cmd_status, causing the if (ret) check to evaluate to true.
Since the reply was successfully received and consumed, there doesn't seem to be
an orphaned ACK, but this will set orphan_ack_pending = true and penalize the
next unrelated command with an unnecessary 25ms delay. Is this the intended
behavior, or should it specifically check for the timeout condition instead?
> +
> return ret;
> }
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260914225303.868569-1-derekjohn.clark@gmail.com?part=2
next prev parent reply other threads:[~2026-09-14 23:26 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-14 22:52 [PATCH 00/10] HID: hid-lenovo-go-s: Fix various bugs Derek J. Clark
2026-09-14 22:52 ` [PATCH 01/10] HID: hid-lenovo-go-s: Return ret instead of 0 in mcu_property_out() Derek J. Clark
2026-09-14 22:52 ` [PATCH 02/10] HID: hid-lenovo-go-s: Bound stale reply window before reusing send_cmd_complete Derek J. Clark
2026-09-14 23:26 ` sashiko-bot [this message]
2026-09-14 22:52 ` [PATCH 03/10] HID: hid-lenovo-go-s: Prevent deadlock if device removed during setup Derek J. Clark
2026-09-14 23:28 ` sashiko-bot
2026-09-14 22:52 ` [PATCH 04/10] HID: hid-lenovo-go-s: Use pm_ptr for reset_resume callback Derek J. Clark
2026-09-14 22:52 ` [PATCH 05/10] HID: hid-lenovo-go-s: Add kobject_uevent notify during reset-resume Derek J. Clark
2026-09-14 22:52 ` [PATCH 06/10] HID: hid-lenovo-go-s: Add suspend function and gate access under bool Derek J. Clark
2026-09-14 23:31 ` sashiko-bot
2026-09-14 22:53 ` [PATCH 07/10] HID: hid-lenovo-go-s: Move attribute init to after device query Derek J. Clark
2026-09-14 23:30 ` sashiko-bot
2026-09-14 22:53 ` [PATCH 08/10] HID: hid-lenovo-go-s: Reorganize and rename hid_gos_cfg Derek J. Clark
2026-09-14 22:53 ` [PATCH 09/10] HID: hid-lenovo-go-s: Move static led_classdev_mc to drvdata struct Derek J. Clark
2026-09-14 22:53 ` [PATCH 10/10] HID: hid-lenovo-go-s: Use devm_kzalloc for drvdata Derek J. Clark
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260914232656.45A751F000FF@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=derekjohn.clark@gmail.com \
--cc=dmitry.torokhov@gmail.com \
--cc=linux-input@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox