Re: [PATCH 18/26] sh: maple: introduce callback_mutex in maple_device
From: sashiko-bot@kernel.org
Date: 2026-07-04 06:14:59
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: New issues: - [Critical] The newly introduced callback_mutex causes a guaranteed self-deadlock when enumerating vmu-flash devices. - [Medium] The newly introduced callback_mutex is initialized dynamically but is not destroyed during the standard device release lifecycle. Pre-existing issues: - [High] The patch fails to prevent the Use-After-Free race condition in the vmu-flash driver because vmu_disconnect bypasses the new callback_mutex. -- commit 29c05632bf3bdcdb9087fb8230fb421ba8176ce5 Author: Dmitry Torokhov [off-list ref] sh: maple: introduce callback_mutex in maple_device This commit introduces callback_mutex in struct maple_device to synchronize callback registration and invocation. It is intended to prevent use-after-free bugs if a device is removed while a callback is in flight.
quoted hunk ↗ jump to hunk
diff --git a/drivers/sh/maple/maple.c b/drivers/sh/maple/maple.c index c0715e3ace6f1..7c82b7a8a2809 100644 --- a/drivers/sh/maple/maple.c +++ b/drivers/sh/maple/maple.c@@ -121,6 +121,7 @@ void maple_getcond_callback(struct maple_device *dev, void (*callback) (struct mapleq *mq), unsigned long interval, unsigned long function) { + guard(mutex)(&dev->callback_mutex);
[Severity: Critical] Does this introduce a self-deadlock when a callback registers a new callback? When enumerating a vmu-flash device, maple_dma_handler() acquires mdev->callback_mutex and executes the callback vmu_queryblocks(). Inside vmu_queryblocks(), the driver calls maple_getcond_callback() to configure the next phase. Since maple_getcond_callback() unconditionally attempts to acquire the same non-recursive mdev->callback_mutex here, won't this immediately deadlock the maple bus workqueue?
dev->callback = callback; dev->interval = interval; dev->function = cpu_to_be32(function);
[ ... ]
quoted hunk ↗ jump to hunk
@@ -230,11 +231,13 @@ static struct maple_device *maple_alloc_dev(int port, int unit) mdev->dev.bus = &maple_bus_type; mdev->dev.parent = &maple_bus; init_waitqueue_head(&mdev->maple_wait); + mutex_init(&mdev->callback_mutex); return mdev; } static void maple_free_dev(struct maple_device *mdev) { + mutex_destroy(&mdev->callback_mutex);
[Severity: Medium] Is a corresponding mutex_destroy() needed in the standard device release path? While mutex_destroy() is added to maple_free_dev() here, if a maple device's reference count drops to zero after device_unregister(), the release path maple_release_device() is executed. Will this leave stale lockdep/RT-mutex structures behind if debugging configs are enabled, since maple_release_device() does not destroy the mutex before calling kfree(mdev)?
kmem_cache_free(maple_queue_cache, mdev->mq->recvbuf); kfree(mdev->mq); kfree(mdev);
[ ... ]
quoted hunk ↗ jump to hunk
@@ -655,8 +658,10 @@ static void maple_dma_handler(struct work_struct *work) break; case MAPLE_RESPONSE_DATATRF: - if (mdev->callback) - mdev->callback(mq); + scoped_guard(mutex, &mdev->callback_mutex) { + if (mdev->callback) + mdev->callback(mq); + }
[Severity: High]
This is a pre-existing issue, but does this actually prevent the
use-after-free race condition for all drivers?
If a user unbinds a vmu-flash device while a maple data transfer is
in-flight, vmu_disconnect() bypasses this new mutex entirely:
drivers/mtd/maps/vmu-flash.c:vmu_disconnect() {
...
mdev->callback = NULL;
card = maple_get_drvdata(mdev);
...
kfree(card);
}
Since vmu_disconnect() modifies the callback and frees the private data
without acquiring callback_mutex, can maple_dma_handler() still
concurrently dereference the freed card data here?
atomic_set(&mdev->busy, 0); wake_up(&mdev->maple_wait); break;
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260703-b4-maple-cleanup-v1-0-41e424964da5@gmail.com?part=18