Re: [PATCH v2] HID: multitouch: use GFP_ATOMIC for report/application allocations

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but lockless modification of the `td->applications` list can race with concurrent iterations, leading to list corruption.
--

commit b30bba4fe7febcb978a1ecb8c9146a4d278538eb
Author: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>

HID: multitouch: use GFP_ATOMIC for report/application allocations

This commit fixes a lockdep warning by changing devm_kzalloc() calls in
mt_allocate_report_data() and mt_allocate_application() to use GFP_ATOMIC.
This resolves the issue where these functions could be called in softirq
context and improperly attempt to sleep.

Reported-by: syzbot+093e05755c2d99caab91@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=093e05755c2d99caab91
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>

> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 451c7324e6a06..5fdc383aadef3 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
[ ... ]
> @@ -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);
>  	if (!mt_application)
>  		return NULL;
>  

[Severity: High]
This is a pre-existing issue, but does lockless modification of the
td->applications list later in this function race with concurrent iterations?

A malicious USB device could send a touch report to arm the
mt_expired_timeout timer, and subsequently send an unmapped feature or input
report. The unmapped report triggers mt_report() -> mt_allocate_application()
in softirq context, mutating the td->applications list via list_add_tail().

Concurrently, the timer callback mt_release_contacts() iterates this same
list locklessly:

mt_release_contacts()
    list_for_each_entry(application, &td->applications, list) {
        ...
    }

Similarly, a local user writing to the quirks sysfs attribute triggers
mt_set_quirks(), which also iterates the list without synchronization.

Could this concurrent modification and iteration corrupt the linked list
pointers and lead to a use-after-free or kernel panic?

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