Re: [PATCH] HID: multitouch: use GFP_ATOMIC for report/application/usage allocations
flat view
From: sashiko-bot@kernel.org
Date: 2026-09-10 15:53:05
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: New issues: - [Medium] `mt_allocate_usage()` is incorrectly assumed to run in softirq context, leading to an unnecessary downgrade to GFP_ATOMIC. Pre-existing issues: - [High] Unprotected iterations of `td->applications` can race with dynamic list additions in softirq context. -- commit 2b43dba239544b5949bf20a7a3f499c51db9632b Author: Nguyen Ngoc Thang [off-list ref] Subject: HID: multitouch: use GFP_ATOMIC for report/application/usage allocations This commit changes the allocation flags for multitouch report data, applications, and usages from GFP_KERNEL to GFP_ATOMIC to prevent lockdep warnings about sleeping in softirq context. Link: https://syzkaller.appspot.com/bug?extid=093e05755c2d99caab91
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index 451c7324e6a06..925b03619d264 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c@@ -604,7 +604,8 @@ static struct mt_usages *mt_allocate_usage(struct hid_device *hdev, { struct mt_usages *usage; - usage = devm_kzalloc(&hdev->dev, sizeof(*usage), GFP_KERNEL); + /* may run from hid_report_raw_event() in (soft)irq context */ + usage = devm_kzalloc(&hdev->dev, sizeof(*usage), GFP_ATOMIC);
[Severity: Medium] Is this downgrade to GFP_ATOMIC necessary? Looking at the call chain, it appears mt_allocate_usage() is only invoked by mt_store_field(), which is used exclusively via the MT_STORE_FIELD macro in mt_touch_input_mapping(). Since mt_touch_input_mapping() is only called during device probe (hid_hw_start()) and is never invoked from mt_event() or any softirq context, using GFP_ATOMIC here might unnecessarily increase the risk of allocation failures during device initialization under memory pressure.
quoted hunk ↗ jump to hunk
if (!usage) return NULL;@@ -633,8 +634,9 @@ static struct mt_application *mt_allocate_application(struct mt_device *td, unsigned int application = report->application; struct mt_application *mt_application; + /* may run from hid_report_raw_event() in (soft)irq context */ mt_application = devm_kzalloc(&td->hdev->dev, sizeof(*mt_application), - GFP_KERNEL); + GFP_ATOMIC);
[Severity: High]
This is a pre-existing issue, but can this allocation and subsequent list
addition race with other list iterations?
When an unknown or unmapped report ID is received, mt_event() can call
mt_allocate_application() from softirq context, which subsequently appends
the new application to td->applications:
drivers/hid/hid-multitouch.c:mt_allocate_application() {
...
list_add_tail(&mt_application->list, &td->applications);
...
}
If this happens concurrently with mt_set_quirks() (running in sysfs process
context) or mt_release_contacts() (running in a timer callback) which both
iterate td->applications without locks:
drivers/hid/hid-multitouch.c:mt_release_contacts() {
...
list_for_each_entry(application, &td->applications, list) {
...
}
Could this concurrent modification and unprotected iteration lead to list
corruption or infinite loops? A malicious or buggy HID device might be able
to trigger this by sending unexpected report IDs.
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260910153746.16222-1-ngocthang2710.1999@gmail.com?part=1