Re: [PATCH v8 03/10] HID: asus: fortify keyboard handshake

[Denis Benato <benato.denis96@gmail.com>]

On 11/19/25 00:46, luke@ljones.dev wrote:
>
>> On 19 Nov 2025, at 12:43, Denis Benato <benato.denis96@gmail.com> wrote:
>>
>>
>> On 11/1/25 11:47, Antheas Kapenekakis wrote:
>>> Handshaking with an Asus device involves sending it a feature report
>>> with the string "ASUS Tech.Inc." and then reading it back to verify the
>>> handshake was successful, under the feature ID the interaction will
>>> take place.
>>>
>>> Currently, the driver only does the first part. Add the readback to
>>> verify the handshake was successful. As this could cause breakages,
>>> allow the verification to fail with a dmesg error until we verify
>>> all devices work with it (they seem to).
>>>
>>> Since the response is more than 16 bytes, increase the buffer size
>>> to 64 as well to avoid overflow errors.
>>>
>>> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
>>> ---
>>> drivers/hid/hid-asus.c | 32 +++++++++++++++++++++++++++++---
>>> 1 file changed, 29 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
>>> index 4676b7f20caf..03f0d86936fc 100644
>>> --- a/drivers/hid/hid-asus.c
>>> +++ b/drivers/hid/hid-asus.c
>>> @@ -48,7 +48,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>>> #define FEATURE_REPORT_ID 0x0d
>>> #define INPUT_REPORT_ID 0x5d
>>> #define FEATURE_KBD_REPORT_ID 0x5a
>>> -#define FEATURE_KBD_REPORT_SIZE 16
>>> +#define FEATURE_KBD_REPORT_SIZE 64
>>> #define FEATURE_KBD_LED_REPORT_ID1 0x5d
>>> #define FEATURE_KBD_LED_REPORT_ID2 0x5e
>>>
>>> @@ -393,14 +393,40 @@ static int asus_kbd_set_report(struct hid_device *hdev, const u8 *buf, size_t bu
>>>
>>> static int asus_kbd_init(struct hid_device *hdev, u8 report_id)
>>> {
>>> +	/*
>>> +	 * The handshake is first sent as a set_report, then retrieved
>>> +	 * from a get_report. They should be equal.
>>> +	 */
>>> 	const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20, 0x54,
>>> 		     0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
>>> +	u8 *readbuf;
>> __free(kfree) = NULL here? Would simplify the code.
>>
>>> 	int ret;
>>>
>>> 	ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
>>> -	if (ret < 0)
>>> -		hid_err(hdev, "Asus failed to send init command: %d\n", ret);
>>> +	if (ret < 0) {
>>> +		hid_err(hdev, "Asus failed to send handshake: %d\n", ret);
>>> +		return ret;
>>> +	}
>>> +
>>> +	readbuf = kzalloc(FEATURE_KBD_REPORT_SIZE, GFP_KERNEL);
>>> +	if (!readbuf)
>>> +		return -ENOMEM;
>>> +
>>> +	ret = hid_hw_raw_request(hdev, report_id, readbuf,
>>> +				 FEATURE_KBD_REPORT_SIZE, HID_FEATURE_REPORT,
>>> +				 HID_REQ_GET_REPORT);
>> See comments on patch 1 (also reported below): not sure if others
>> report_id are going to answer, my guess is that we will have to try
>> if you choose to go that route.
>>> +	if (ret < 0) {
>>> +		hid_err(hdev, "Asus failed to receive handshake ack: %d\n", ret);
>>> +	} else if (memcmp(readbuf, buf, sizeof(buf)) != 0) {
>>> +		hid_warn(hdev, "Asus handshake returned invalid response: %*ph\n",
>>> +			FEATURE_KBD_REPORT_SIZE, readbuf);
>>> +		/*
>>> +		 * Do not return error if handshake is wrong until this is
>>> +		 * verified to work for all devices.
>>> +		 */
>> In review of patch 1 I requested this function to be called with more report_id
>> than just 0x5a so this will have to be checked against those values too.
>>
>> In alternative you can fork based on the report_id, but having confirmation that
>> this is valid with those ids too would be of great help. Perhaps I can help you
>> with this asking to asus-linux users.
> The handshake works for 0x5D and 0x5E also.
Then this, like patch 2, should have been sent regardless of
this patchset, this whole discussion stalled two good patches
that we all agree are good and are totally independent from
the main issue?
>>> +	}
>>>
>>> +	kfree(readbuf);
>>> 	return ret;
>>> }
>>>