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

[PATCH 6.6.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.6.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] Unbounded user input in sysfs 'delay' attribute allows arbitrary uninterruptible sleep, hanging USB unbind and device hotplug.
--

commit ba9154b0332977963ad5bed995346ca93b2e47dc
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>

Input: aiptek - validate raw macro indices before updating state

This commit adds bounds checking for raw macro key indices derived from
tablet reports before updating the state in the driver. It ensures the
index does not exceed the macroKeyEvents array bounds to prevent out of
bounds access.

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, and was not introduced by the patch under
review, but while reviewing this file I noticed unbounded user input in the
sysfs delay attribute allows an arbitrary uninterruptible sleep.

In drivers/input/tablet/aiptek.c:store_tabletProgrammableDelay():

    err = kstrtoint(buf, 10, &d);
    if (err)
        return err;

    aiptek->newSetting.programmableDelay = d;

User input is parsed into an integer and stored without bounds checking.
When the execute attribute is subsequently written, this value is used
in aiptek_query():

    msleep(aiptek->curSetting.programmableDelay);

Can this cause a task to block indefinitely in TASK_UNINTERRUPTIBLE?

If a user writes a huge positive or negative integer to the delay sysfs
attribute, the unbounded value is passed to msleep(). While holding a sysfs
active reference, this prevents the sysfs node from being removed.

If this is triggered and the USB device is subsequently disconnected, does
this force the kernel's USB hub event thread to hang indefinitely in
kernfs_drain()? This would deadlock the USB hub and prevent further device
enumeration or disconnection.

Should this sysfs input be clamped to a safe maximum value before being
stored in programmableDelay?

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

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

[Greg KH]

On Mon, Sep 07, 2026 at 08:50:01PM +0200, Miguel Garcia wrote:
> 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(-)

What about 6.12.y and 6.18.y?

thanks,

greg k-h

Re: [PATCH 6.6.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.6, thanks.

-- 
Thanks,
Sasha

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

[Miguel García Román]

Hi Greg,

You are right, 6.12.y and 6.18.y need the fix too. I missed those branches
in the initial submission. I am sending the two backports as follow-ups.

Both apply without code changes from upstream commit
95dffe32a66cbed07fbfa7afed39d56d5014e04f. I built
drivers/input/tablet/aiptek.o with W=1 on 6.12.109 and 6.18.50;
both builds passed without warnings.

Both patched kernels also booted under QEMU/KVM with UBSAN enabled
and passed 20 module load/unload cycles. This was a module lifecycle
smoke test, not a test of tablet reports or reproduction of the bug.

Thanks,
Miguel

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

[Miguel García Román]

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

commit 95dffe32a66cbed07fbfa7afed39d56d5014e04f upstream.

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>
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 2b3fbb0455d5c..9e78105bace77 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

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

[Miguel García Román]

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

commit 95dffe32a66cbed07fbfa7afed39d56d5014e04f upstream.

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>
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 2b3fbb0455d5c..9e78105bace77 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.12.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] Sysfs attribute handlers dereference NULL driver data during device probe.
--

commit 645c2a36ee70a10a323ece648cef25c5a29065c9
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>

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[]. 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 2b3fbb0455d5c..9e78105bace77 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);

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but it is a pre-existing issue.
Can this cause a NULL pointer dereference during device probe?

In aiptek_probe(), there is a window where the driver core adds dev_groups
(making sysfs attributes visible to user-space) before usb_set_intfdata()
is called.

If a user-space process concurrently reads or writes the sysfs attributes
during this window, dev_get_drvdata() will return NULL.

This blindly dereferences the NULL aiptek pointer in show_tabletSize():

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

	return sysfs_emit(buf, "%dx%d\n",
			  input_abs_get_max(aiptek->inputdev, ABS_X) + 1,
			  input_abs_get_max(aiptek->inputdev, ABS_Y) + 1);
}

The same blind dereference occurs in the write path of store_tabletExecute():

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

	/* We do not care what you write to this file. Merely the action
	 * of writing to this file triggers a tablet reprogramming.
	 */
	memcpy(&aiptek->curSetting, &aiptek->newSetting,
	       sizeof(struct aiptek_settings));
    ...
}

And also when reading the firmware code in show_firmwareCode():

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

	return sysfs_emit(buf, "%04x\n", aiptek->features.firmwareCode);
}

Since these sysfs files are created with S_IRUGO (0444), any unprivileged
local user could trigger this panic by repeatedly reading the files during
device plug.

Could the sysfs group creation be deferred until after usb_set_intfdata()
has completed its initialization?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/CABKbRo+qUMG8=+-MnY34KkscgZdPASieN7CgWce9BTCggNSvcg@mail.gmail.com?part=1