Skip to content

Import multiple upstream patches #463

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jul 16, 2025
Merged

Conversation

Joshua-Dickens
Copy link
Member

@Joshua-Dickens Joshua-Dickens commented Jul 15, 2025

Importing multiple upstream patches that cover:

  • Handling the rename of del_timer_sync to timer_delete_sync
  • Various Memory Leak and Allocation fixes
  • Fixing a crash that was caused in wacom_aes_battery_handler

As well as adding a new flag to configure.ac (WACOM_TIMER_DELETE_SYNC) for use in wacom_sys.c to handle kernel versions before the rename (<6.1.84) and ensure it compiles correctly on those systems.

KAGA-KOKO and others added 9 commits July 14, 2025 23:51
timer_delete[_sync]() replaces del_timer[_sync](). Convert the whole tree
over and remove the historical wrapper inlines.

Conversion was done with coccinelle plus manual fixups where necessary.

Signed-off-by: Thomas Gleixner <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
[[email protected]: Imported into input-wacom (8fa7292fee5c)]
Signed-off-by: Joshua Dickens <[email protected]>
…ush()

During wacom_wac_queue_flush() the code calls
kzalloc() to allocate a zero initialised buffer
which it uses as a storage buffer to get data
from the fifo via kfifo_out(). However it does not
check kzalloc() for allocation failure which returns
NULL and could potentially lead to a NULL deref.

Fix this by checking for kzalloc() failure and skipping
the current entry if allocation failure occurs.

Fixes: 5e013ad20689 ("HID: wacom: Remove static WACOM_PKGLEN_MAX limit")
Signed-off-by: Qasim Ijaz <[email protected]>
Reviewed-by: Jason Gerecke <[email protected]>
Signed-off-by: Jiri Kosina <[email protected]>
[[email protected]: Imported into input-wacom (e1ca5f39c2e3)]
Signed-off-by: Joshua Dickens <[email protected]>
In wacom_wac_queue_flush() the code allocates zero initialised
buffer which it uses as a storage buffer for copying data from
a fifo via kfifo_out(). The kfifo_out() function returns the
number of elements it has copied. The code checks if the number
of copied elements does not equal the size of the fifo record,
if it does not it simply skips the entry and continues to the
next iteration. However it does not release the storage buffer
leading to a memory leak.

Fix the memory leak by freeing the buffer on size mismatch.

Fixes: 5e013ad20689 ("HID: wacom: Remove static WACOM_PKGLEN_MAX limit")
Reviewed-by: Jason Gerecke <[email protected]>
Signed-off-by: Qasim Ijaz <[email protected]>
Signed-off-by: Jiri Kosina <[email protected]>
[[email protected]: Imported into input-wacom (fd34bf79a617)]
Signed-off-by: Joshua Dickens <[email protected]>
During wacom_parse_and_register() the code calls wacom_devm_kfifo_alloc
to allocate a fifo. During this operation it passes kfifo_alloc a
fifo_size of 0. Kfifo attempts to round the size passed to it to the
next power of 2 via roundup_pow_of_two (queue-type data structures
do this to maintain efficiency of operations).

However during this phase a problem arises when the roundup_pow_of_two()
function utilises a shift exponent of fls_long(n-1), where n is the
fifo_size. Since n is 0 in this case and n is also an unsigned long,
doing n-1 causes unsigned integer wrap-around to occur making the
fifo_size 4294967295. So the code effectively does fls_long(4294967295)
which results in 64. Returning back to roundup_pow_of_two(), the code
utilises a shift exponent of 64. When a shift exponent of 64 is used
on a 64-bit type such as 1UL it results in a shift-out-of-bounds.

The root cause of the issue seems to stem from insufficient validation
of wacom_compute_pktlen(), since in this case the fifo_size comes
from wacom_wac->features.pktlen. During wacom_parse_and_register()
the wacom_compute_pktlen() function sets the pktlen as 0.

To fix this, we should handle cases where wacom_compute_pktlen()
results in 0.

Reported-by: syzbot <[email protected]>
Closes: https://syzkaller.appspot.com/bug?extid=d5204cbbdd921f1f7cad
Fixes: 5e013ad20689 ("HID: wacom: Remove static WACOM_PKGLEN_MAX limit")
Tested-by: Qasim Ijaz <[email protected]>
Reviewed-by: Jason Gerecke <[email protected]>
Cc: [email protected]
Signed-off-by: Qasim Ijaz <[email protected]>
Signed-off-by: Jiri Kosina <[email protected]>
[[email protected]: Imported into input-wacom (6bf8ab7774a2)]
Signed-off-by: Joshua Dickens <[email protected]>
During wacom_initialize_remotes() a fifo buffer is allocated
with kfifo_alloc() and later a cleanup action is registered
during devm_add_action_or_reset() to clean it up.

However if the code fails to create a kobject and register it
with sysfs the code simply returns -ENOMEM before the cleanup
action is registered leading to a memory leak.

Fix this by ensuring the fifo is freed when the kobject creation
and registration process fails.

Fixes: 83e6b40e2de6 ("HID: wacom: EKR: have the wacom resources dynamically allocated")
Reviewed-by: Ping Cheng <[email protected]>
Cc: [email protected]
Signed-off-by: Qasim Ijaz <[email protected]>
Signed-off-by: Jiri Kosina <[email protected]>
[[email protected]: Imported into input-wacom (5ae416c5b1e2)]
Signed-off-by: Joshua Dickens <[email protected]>
When sysfs_create_files() fails during wacom_initialize_remotes() the
fifo buffer is not freed leading to a memory leak.

Fix this by calling kfifo_free() before returning.

Fixes: 83e6b40e2de6 ("HID: wacom: EKR: have the wacom resources dynamically allocated")
Reviewed-by: Ping Cheng <[email protected]>
Cc: [email protected]
Signed-off-by: Qasim Ijaz <[email protected]>
Signed-off-by: Jiri Kosina <[email protected]>
[[email protected]: Imported into input-wacom (1a19ae437ca5)]
Signed-off-by: Joshua Dickens <[email protected]>
When sysfs_create_files() fails in wacom_initialize_remotes() the error
is returned and the cleanup action will not have been registered yet.

As a result the kobject???s refcount is never dropped, so the
kobject can never be freed leading to a reference leak.

Fix this by calling kobject_put() before returning.

Fixes: 83e6b40e2de6 ("HID: wacom: EKR: have the wacom resources dynamically allocated")
Acked-by: Ping Cheng <[email protected]>
Cc: [email protected]
Signed-off-by: Qasim Ijaz <[email protected]>
Signed-off-by: Jiri Kosina <[email protected]>
[[email protected]: Imported into input-wacom (85a720f4337f)]
Signed-off-by: Joshua Dickens <[email protected]>
Commit fd2a9b29dc9c ("HID: wacom: Remove AES power_supply after extended
inactivity") introduced wacom_aes_battery_handler() which is scheduled
as a delayed work (aes_battery_work).

In wacom_remove(), aes_battery_work is not canceled. Consequently, if
the device is removed while aes_battery_work is still pending, then hard
crashes or "Oops: general protection fault..." are experienced when
wacom_aes_battery_handler() is finally called. E.g., this happens with
built-in USB devices after resume from hibernate when aes_battery_work
was still pending at the time of hibernation.

So, take care to cancel aes_battery_work in wacom_remove().

Fixes: fd2a9b29dc9c ("HID: wacom: Remove AES power_supply after extended inactivity")
Signed-off-by: Thomas Zeitlhofer <[email protected]>
Acked-by: Ping Cheng <[email protected]>
Signed-off-by: Jiri Kosina <[email protected]>
[[email protected]: Imported into input-wacom (f3054152c12e)]
Signed-off-by: Joshua Dickens <[email protected]>
In kernel ver 6.1.84 del_timer_sync was renamed to timer_delete_sync.
This commit adds a check and flag set in configure.ac for this change.
It also adds an #ifdef check in wacom_sys.c to handle the flag.

Resolves: linuxwacom#461
Signed-off-by: Joshua Dickens <[email protected]>
@Pinglinux Pinglinux merged commit c6b7540 into linuxwacom:master Jul 16, 2025
8 of 9 checks passed
@Pinglinux
Copy link
Member

The PR looks good to me. Thank you @Joshua-Dickens!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants