Re: [PATCH 15/26] sh: maple: remove not needed maple_unsupported_device driver
From: sashiko-bot@kernel.org
Date: 2026-07-04 06:11:36
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: Pre-existing issues: - [High] Asynchronous work items are not canceled on the `maple_bus_init` error path. - [High] Error path in `maple_bus_init` frees devices that are still linked on the global `maple_waitq`. - [High] Directly calling `kfree` via `maple_free_dev` on `device_register` failure in `maple_attach_driver` leaks kobject state. - [High] Re-queuing a maple packet without setting the busy flag causes list corruption. -- commit 8ed68bea570556010f84062b91e5bd48da1f0c8d Author: Dmitry Torokhov [off-list ref] sh: maple: remove not needed maple_unsupported_device driver This commit removes the maple_unsupported_device fallback driver since the driver core handles devices without drivers. It also updates the shared interrupt registrations to use &maple_bus instead of the removed driver.
quoted hunk ↗ jump to hunk
diff --git a/drivers/sh/maple/maple.c b/drivers/sh/maple/maple.c --- a/drivers/sh/maple/maple.c +++ b/drivers/sh/maple/maple.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -378,7 +376,6 @@ static void maple_attach_driver(struct maple_device *mdev) if (matched == 0) { /* Driver does not exist yet */ dev_info(&mdev->dev, "no driver found\n"); - mdev->driver = &maple_unsupported_device; } dev_set_name(&mdev->dev, "%d:0%d.%lX", mdev->port, mdev->unit, function);
[Severity: High]
This isn't a regression introduced by this patch, but does the error path in
maple_attach_driver() leak kobject state?
If device_register(&mdev->dev) fails:
error = device_register(&mdev->dev);
if (error) {
dev_warn(&mdev->dev, "could not register device at"
" (%d, %d), with error 0x%X\n", mdev->unit,
mdev->port, error);
maple_free_dev(mdev);
mdev = NULL;
return;
}
maple_free_dev() directly calls kfree(). Since device_register() was
called, shouldn't failures be handled by calling put_device() to safely
invoke the release callback instead of bypassing it?
[ ... ]
quoted hunk ↗ jump to hunk
@@ -863,17 +851,14 @@ static int __init maple_bus_init(void) kmem_cache_destroy(maple_queue_cache); cleanup_bothirqs: - free_irq(HW_EVENT_VSYNC, 0); + free_irq(HW_EVENT_VSYNC, &maple_bus); cleanup_irq: - free_irq(HW_EVENT_MAPLE_DMA, 0); + free_irq(HW_EVENT_MAPLE_DMA, &maple_bus); cleanup_dma: free_pages((unsigned long) maple_sendbuf, MAPLE_DMA_PAGES);
[Severity: High]
This is a pre-existing issue, but can this error path lead to a
use-after-free crash?
In maple_bus_init(), if initialization fails after setting the DMA
interrupt handler (for example, if KMEM_CACHE allocation fails), the error
path calls free_irq() and frees the maple_sendbuf memory.
However, it does not appear to call cancel_work_sync() for
maple_dma_process and maple_vblank_process. If an interrupt fired before
free_irq(), could the scheduled work items execute asynchronously after the
buffers are freed, dereferencing the freed memory?
[Severity: High]
This is another pre-existing issue in maple_bus_init(), but does the port
initialization loop leave dangling pointers on the global maple_waitq?
During the loop, each allocated device is immediately queued onto the
global maple_waitq via maple_add_packet():
for (i = 0; i < MAPLE_PORTS; i++) {
checked[i] = false;
empty[i] = false;
mdev[i] = maple_alloc_dev(i, 0);
if (!mdev[i]) {
while (i-- > 0)
maple_free_dev(mdev[i]);
If a subsequent allocation fails, the error handler loops backwards and
calls maple_free_dev(). However, maple_free_dev() frees mdev and mdev->mq
directly via kfree() without executing list_del() to remove them from
maple_waitq. Could this corrupt the linked list?
[Severity: High]
This is a pre-existing issue not touched by this patch, but can re-queuing
a maple packet without setting the busy flag cause list corruption?
Looking at setup_maple_commands() in drivers/sh/maple/maple.c, a
MAPLE_COMMAND_GETCOND packet is queued if mdev->busy is 0:
if (mdev->interval > 0 && atomic_read(&mdev->busy) == 0 &&
time_after(jiffies, mdev->when)) {
/* bounce if we cannot add */
add = maple_add_packet(mdev,
be32_to_cpu(mdev->devinfo.function),
MAPLE_COMMAND_GETCOND, 1, NULL);
if (!add)
mdev->when = jiffies + mdev->interval;
}
It successfully calls maple_add_packet(), which links mdev->mq->list into
maple_waitq. However, unlike other packet types, it fails to set
mdev->busy = 1.
If a concurrent userspace action (such as a read via /dev/mtdX in vmu-flash
leading to maple_vmu_read_block()) executes, would it read mdev->busy as 0,
assume the queue is free, and call maple_add_packet() again? This would
reuse the exact same mdev->mq object, executing list_add_tail() on an
already-linked node, which corrupts the list.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703-b4-maple-cleanup-v1-0-41e424964da5@gmail.com?part=15