Re: [PATCH 1/2] xtensa/simdisk: Avoid referring to module::args
From: sashiko-bot@kernel.org
Date: 2026-06-04 11:18:32
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] Concurrent access to `dev->filename` in `proc_read_simdisk` and `simdisk_detach` causes a Use-After-Free. -- commit e0a519c0b452f39fc91f2ac38f46b81a32db738a Author: Petr Pavlu [off-list ref] xtensa/simdisk: Avoid referring to module::args This commit prepares for the removal of module::args by copying the filename string provided as a parameter instead of storing a direct pointer when built as a loadable module. It also adds a free function to clean up these copied strings upon module unload.
quoted hunk ↗ jump to hunk
diff --git a/arch/xtensa/platforms/iss/simdisk.c b/arch/xtensa/platforms/iss/simdisk.c index 7c7a2aa749f82..a2ddb49c269dc 100644 --- a/arch/xtensa/platforms/iss/simdisk.c +++ b/arch/xtensa/platforms/iss/simdisk.c
[ ... ]
static const struct kernel_param_ops simdisk_param_ops_filename = {
.set = simdisk_param_set_filename,
+ .free = simdisk_param_free_filename,
};
-module_param_cb(filename, &simdisk_param_ops_filename, &n_files, 0);
+module_param_cb(filename, &simdisk_param_ops_filename, NULL, 0);
MODULE_PARM_DESC(filename, "Backing storage filename.");
static int simdisk_major = SIMDISK_MAJOR;
[Severity: High]
This isn't a bug introduced by this patch, but is it possible to trigger a
use-after-free here due to concurrent access to dev->filename?
The procfs entry for simdisk devices is created with 0644 permissions, allowing
any user to read it. In proc_read_simdisk(), dev->filename is fetched without
acquiring dev->lock:
arch/xtensa/platforms/iss/simdisk.c:proc_read_simdisk() {
...
const char *s = dev->filename;
if (s) {
ssize_t len = strlen(s);
char *temp = kmalloc(len + 2, GFP_KERNEL);
...
}
If an administrator concurrently writes to the same procfs entry, it triggers
proc_write_simdisk() -> simdisk_detach(), which acquires dev->lock and frees
the filename:
arch/xtensa/platforms/iss/simdisk.c:simdisk_detach() {
...
spin_lock(&dev->lock);
...
} else {
pr_info("SIMDISK: %s detached from %s\n",
dev->gd->disk_name, dev->filename);
dev->fd = -1;
kfree(dev->filename);
dev->filename = NULL;
}
...
}
If proc_read_simdisk() fetches the pointer right before simdisk_detach() frees
it, the subsequent strlen() and scnprintf() operations could dereference freed
memory. Does this need locking or RCU protection to prevent a potential oops
or local information leak?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260604110455.1608038-1-petr.pavlu@suse.com?part=1