[PATCH 6.1.y] Input: aiptek - validate raw macro indices before updating state

[PATCH 6.1.y] Input: aiptek - validate raw macro indices before updating state

[Miguel Garcia]

From: Pengpeng Hou <pengpeng@iscas.ac.cn>

aiptek_irq() derives macro key indices directly from tablet reports and
then uses them to index macroKeyEvents[]. Report types 4 and 5 also save
the derived value in aiptek->lastMacro and later use that state to
release the previous key.

Validate the raw macro index once before it enters that state machine, so
lastMacro only ever stores an in-range macro key. Keep direct bounds
checks for report type 6, which reads the macro number from the packet
body and uses it immediately.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Link: https://patch.msgid.link/20260329001711.88076-1-pengpeng@iscas.ac.cn
[dtor: fix macro fallback in report 5s to use -1]
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
(cherry picked from commit 95dffe32a66cbed07fbfa7afed39d56d5014e04f)
Signed-off-by: Miguel Garcia <miguelgarciaroman8@gmail.com>
---
 drivers/input/tablet/aiptek.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/input/tablet/aiptek.c b/drivers/input/tablet/aiptek.c
index baabc51547b83..6210cd99d6291 100644
--- a/drivers/input/tablet/aiptek.c
+++ b/drivers/input/tablet/aiptek.c
@@ -658,6 +658,8 @@ static void aiptek_irq(struct urb *urb)
 		pck = (data[1] & aiptek->curSetting.stylusButtonUpper) != 0 ? 1 : 0;
 
 		macro = dv && p && tip && !(data[3] & 1) ? (data[3] >> 1) : -1;
+		if (macro >= ARRAY_SIZE(macroKeyEvents))
+			macro = -1;
 		z = get_unaligned_le16(data + 4);
 
 		if (dv) {
@@ -699,7 +701,9 @@ static void aiptek_irq(struct urb *urb)
 		left = (data[1]& aiptek->curSetting.mouseButtonLeft) != 0 ? 1 : 0;
 		right = (data[1] & aiptek->curSetting.mouseButtonRight) != 0 ? 1 : 0;
 		middle = (data[1] & aiptek->curSetting.mouseButtonMiddle) != 0 ? 1 : 0;
-		macro = dv && p && left && !(data[3] & 1) ? (data[3] >> 1) : 0;
+		macro = dv && p && left && !(data[3] & 1) ? (data[3] >> 1) : -1;
+		if (macro >= ARRAY_SIZE(macroKeyEvents))
+			macro = -1;
 
 		if (dv) {
 		        /* If the selected tool changed, reset the old
@@ -737,11 +741,11 @@ static void aiptek_irq(struct urb *urb)
 	 */
 	else if (data[0] == 6) {
 		macro = get_unaligned_le16(data + 1);
-		if (macro > 0) {
+		if (macro > 0 && macro - 1 < ARRAY_SIZE(macroKeyEvents)) {
 			input_report_key(inputdev, macroKeyEvents[macro - 1],
 					 0);
 		}
-		if (macro < 25) {
+		if (macro + 1 < ARRAY_SIZE(macroKeyEvents)) {
 			input_report_key(inputdev, macroKeyEvents[macro + 1],
 					 0);
 		}
@@ -760,7 +764,8 @@ static void aiptek_irq(struct urb *urb)
 				aiptek->curSetting.toolMode;
 		}
 
-		input_report_key(inputdev, macroKeyEvents[macro], 1);
+		if (macro < ARRAY_SIZE(macroKeyEvents))
+			input_report_key(inputdev, macroKeyEvents[macro], 1);
 		input_report_abs(inputdev, ABS_MISC,
 				 1 | AIPTEK_REPORT_TOOL_UNKNOWN);
 		input_sync(inputdev);
-- 
2.43.0

Re: [PATCH 6.1.y] Input: aiptek - validate raw macro indices before updating state

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Concurrent access to sysfs attributes during device unbind leads to NULL pointer dereference or Use-After-Free.
--

commit e5588eae75b51cb3ffbe74aca6501e040a49e90c
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>
Date:   Mon Sep 7 18:49:59 2026 +0000

    Input: aiptek - validate raw macro indices before updating state
    
    aiptek_irq() derives macro key indices directly from tablet reports and
    then uses them to index macroKeyEvents[]. Report types 4 and 5 also save
    the derived value in aiptek->lastMacro and later use that state to
    release the previous key.
    
    Validate the raw macro index once before it enters that state machine, so
    lastMacro only ever stores an in-range macro key. Keep direct bounds
    checks for report type 6, which reads the macro number from the packet
    body and uses it immediately.
    
    Link: https://patch.msgid.link/20260329001711.88076-1-pengpeng@iscas.ac.cn

> diff --git a/drivers/input/tablet/aiptek.c b/drivers/input/tablet/aiptek.c
> index baabc51547b83..6210cd99d6291 100644
> --- a/drivers/input/tablet/aiptek.c
> +++ b/drivers/input/tablet/aiptek.c

[ ... ]

[Severity: High]
This is a pre-existing issue, but can concurrent access to sysfs attributes
during device unbind lead to a NULL pointer dereference or use-after-free?

Looking at aiptek_disconnect(), the driver data is cleared and the aiptek
structure is freed:

drivers/input/tablet/aiptek.c:aiptek_disconnect() {
    ...
	/* Disassociate driver's struct with usb interface
	 */
	usb_set_intfdata(intf, NULL);
	if (aiptek != NULL) {
		/* Free & unhook everything from the system.
		 */
		usb_kill_urb(aiptek->urb);
		...
		kfree(aiptek);
	}
}

Since the USB driver core removes sysfs files after the disconnect callback
returns, world-readable sysfs handlers like show_tabletToolMode() remain
accessible and can race with the teardown:

drivers/input/tablet/aiptek.c:show_tabletToolMode() {
	struct aiptek *aiptek = dev_get_drvdata(dev);

	return sysfs_emit(buf, "%s\n", map_val_to_str(tool_mode_map,
						      aiptek->curSetting.toolMode));
}

If a sysfs reader calls dev_get_drvdata() after usb_set_intfdata(intf, NULL)
has been executed, it will fetch a NULL pointer and dereference it without
checks.

Alternatively, if the reader fetches the pointer just before it is cleared
but accesses it after kfree(aiptek) finishes, couldn't this trigger a
use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907184959.1567893-1-miguelgarciaroman8@gmail.com?part=1

Re: [PATCH 6.1.y] Input: aiptek - validate raw macro indices before updating state

[Sasha Levin]

> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> Link: https://patch.msgid.link/20260329001711.88076-1-pengpeng@iscas.ac.cn
> [dtor: fix macro fallback in report 5s to use -1]
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> (cherry picked from commit 95dffe32a66cbed07fbfa7afed39d56d5014e04f)
> Signed-off-by: Miguel Garcia <miguelgarciaroman8@gmail.com>

This is upstream 95dffe32a66c ("Input: aiptek - validate raw macro indices
before updating state"), authored by Pengpeng Hou and adapted by Dmitry
Torokhov. Your diff matches it byte-for-byte. Before I queue it under your
Signed-off-by, can you confirm your relationship to this submission (sending on
Pengpeng's/Dmitry's behalf, or an independent resubmission)? Want to get the
attribution right before it goes in.

-- 
Thanks,
Sasha

Re: [PATCH 6.1.y] Input: aiptek - validate raw macro indices before updating state

[miguelgarciaroman8]

> Signed-off-by, can you confirm your relationship to this submission (sending on
> Pengpeng's/Dmitry's behalf, or an independent resubmission)?

This is an independent submission for stable, not on their behalf.
Pengpeng is the author, with Dmitry's adjustment. I forwarded the upstream
patch unchanged and added my Signed-off-by as the stable submitter.

Thanks,
Miguel

Re: [PATCH 6.1.y] Input: aiptek - validate raw macro indices before updating state

[Sasha Levin]

> aiptek_irq() derives macro key indices directly from tablet reports and
> then uses them to index macroKeyEvents[]. Report types 4 and 5 also save
> the derived value in aiptek->lastMacro and later use that state to
> release the previous key.

Queued for 6.1, thanks.

-- 
Thanks,
Sasha