Re: [PATCH 01/26] sh: maple: include linux/device.h in linux/maple.h

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The error path in `vmu_queryblocks` frees the `priv` field of previously registered MTD partitions without unregistering them, leaving dangling MTD devices that crash the kernel if accessed.
- [High] `vmu_disconnect` assumes all partitions have a fully initialized `priv` structure and unconditionally dereferences it, which causes a NULL pointer dereference if initialization failed or was incomplete.
- [High] `vmu_disconnect` fails to free multiple dynamically allocated structures, causing a severe memory leak on device removal.
- [High] `vmu_flash_read_char` truncates negative error codes into an `unsigned char`, causing the caller to silently misinterpret errors as valid data.
--

commit 3694a7f48be1fa76db703e9eae953d9ee9757f9f
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

sh: maple: include linux/device.h in linux/maple.h

This commit includes linux/device.h in linux/maple.h to ensure full
definitions of struct device and struct device_driver are available.
This fixes a compilation error in drivers/mtd/maps/vmu-flash.c.

> diff --git a/include/linux/maple.h b/include/linux/maple.h
> index 3be4e567473c8..22f2930251edc 100644
> --- a/include/linux/maple.h
> +++ b/include/linux/maple.h
[ ... ]

While this patch fixes the build, looking at drivers/mtd/maps/vmu-flash.c
revealed a few pre-existing issues:

drivers/mtd/maps/vmu-flash.c:vmu_flash_read_char() {
...
	unsigned char *buf, ret;
...
	error = maple_vmu_read_block(vblock->num, buf, mtd);
	if (error) {
		ret = error;
		*retval = 2;
		goto out_vblock;
	}
...
	return ret;
}

[Severity: High]
This is a pre-existing issue, but does assigning a negative error code
like -ENOMEM or -EIO to the unsigned char ret truncate it? This could
cause the caller to misinterpret errors as valid data instead of properly
propagating the negative error code back to the MTD subsystem.

drivers/mtd/maps/vmu-flash.c:vmu_queryblocks() {
...
	error = mtd_device_register(mtd_cur, NULL, 0);
	if (error)
		goto fail_mtd_register;
...
fail_mpart:
	for (error = 0; error <= card->partition; error++) {
		kfree(((card->mtd)[error]).priv);
		((card->mtd)[error]).priv = NULL;
	}
...
}

[Severity: High]
This isn't a bug introduced by this patch, but could this cleanup loop
fail to unregister the previously successfully registered MTD devices?
If a recursive call fails, freeing the priv field without calling
mtd_device_unregister() might leave dangling MTD devices that could
crash the kernel if accessed later.

drivers/mtd/maps/vmu-flash.c:vmu_disconnect() {
...
	for (x = 0; x < card->partitions; x++) {
		mpart = ((card->mtd)[x]).priv;
		mpart->mdev = NULL;
		mtd_device_unregister(&((card->mtd)[x]));
		kfree(((card->parts)[x]).name);
	}
...
}

[Severity: High]
This is a pre-existing issue, but does this assume all partitions have a
fully initialized priv structure? If the device is disconnected before
vmu_queryblocks() finishes initialization or if initialization fails,
can mpart be NULL here, leading to a NULL pointer dereference when
assigning mpart->mdev = NULL?

[Severity: High]
This is also a pre-existing issue, but does this disconnect routine leak
resources? It appears to omit freeing the mpart, pcache, and pcache->buffer
resources that were allocated during initialization.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703-b4-maple-cleanup-v1-0-41e424964da5@gmail.com?part=1