Skip to contentSkip to conversation

mtd: maps: vmu-flash: Fix build and runtime errors

PATCH Superseded Florian Fuchs 2 participants 6 messages Apr 27

Subsystems (2)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

v2 cover mtd: maps: vmu-flash: Fix build and runtime errors

Hi all,

This small series fixes build and runtime errors in the vmu-flash driver (enabled by CONFIG_MTD_VMU) and the included maple.h. These changes were verified on real Dreamcast hardware with a physical VMU. The VMU can now be successfully probed, read and written with MTD tools like mtdinfo and mtd_debug. Previously, the driver failed to build or crashed during probing.

bash-5.3# mtdinfo /dev/mtd0
mtd0
Name: vmu2.1.0
Type: mlc-nand
Eraseblock size: 512 bytes
Amount of eraseblocks: 256 (131072 bytes, 128.0 KiB)
Minimum input/output unit size: 512 bytes
Sub-page size: 512 bytes
Character device major/minor: 90:0
Bad blocks are allowed: true
Device is writable: true

Thanks,
Florian
---
v1->v2: Rebased patches and fixed the hotplug issues in other sh
patches.

v1: https://lore.kernel.org/linux-mtd/[email protected]/

The original v1 was postponed to fix the hotplug issue and other runtime panics, that are fixed in the following patch:
"[PATCH v2] sh: maple: fix empty port handling":
https://lore.kernel.org/linux-sh/[email protected]/

Note that in order to use the VMUs while there are empty ports, it requires the above patch.

Florian Fuchs (3):
mtd: maps: vmu-flash: fix build error due to missing include of
linux/device.h
mtd: maps: vmu-flash: fix fault in unaligned fixup
mtd: maps: vmu-flash: Fix NULL pointer dereference in initialization

 drivers/mtd/maps/vmu-flash.c | 7 ++++---
 include/linux/maple.h        | 3 +--
 2 files changed, 5 insertions(+), 5 deletions(-)

base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f

Signature · 1 line
2.43.0
#1 lore raw

PATCH v2 1/3 mtd: maps: vmu-flash: fix build error due to missing include of linux/device.h

Fix build error by adding the missing include of linux/device.h linux/maple.h embeds struct device via struct maple_device, which requires the definition. Otherwise results in build error:

./include/linux/maple.h:81:16: error: field 'dev' has incomplete type

  struct device dev;
                ^~~
./include/linux/maple.h:86:23: error: field 'drv' has incomplete type
  struct device_driver drv;
                       ^~~
Fixes 313162d0b838 ("device.h: audit and cleanup users in main include dir")

---
v1->v2: no functional change, just rebase patch

v1: https://lore.kernel.org/lkml/[email protected]/

 include/linux/maple.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
include/linux/maple.h +1 -2
index 3be4e567473c..22f2930251ed 100644
@@ -2,10 +2,9 @@
 #ifndef __LINUX_MAPLE_H
 #define __LINUX_MAPLE_H
 
+#include <linux/device.h>
 #include <mach/maple.h>
 
-struct device;
-
 /* Maple Bus command and response codes */
 enum maple_code {
 	MAPLE_RESPONSE_FILEERR =	-5,
Signature · 1 line
2.43.0
#2 lore raw

PATCH v2 2/3 mtd: maps: vmu-flash: fix fault in unaligned fixup

In reply to Florian Fuchs

Use kzalloc_obj() / kzalloc_objs() to allocate the memcard structs, instead of kmalloc_obj() / kmalloc_objs() to prevent access to uninitialized data.

Fixes runtime error: Fault in unaligned fixup: 0000 [#1] at mtd_get_fact_prot_info.

---
v1->v2: no functional change, just rebased.

v1: https://lore.kernel.org/lkml/[email protected]/

 drivers/mtd/maps/vmu-flash.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/mtd/maps/vmu-flash.c +3 -3
index 75e06d249ce9..b76d0609d1b7 100644
@@ -609,7 +609,7 @@ static int vmu_connect(struct maple_device *mdev)
 
 	basic_flash_data = be32_to_cpu(mdev->devinfo.function_data[c - 1]);
 
-	card = kmalloc_obj(struct memcard);
+	card = kzalloc_obj(struct memcard);
 	if (!card) {
 		error = -ENOMEM;
 		goto fail_nomem;
@@ -627,13 +627,13 @@ static int vmu_connect(struct maple_device *mdev)
 	* Not sure there are actually any multi-partition devices in the
 	* real world, but the hardware supports them, so, so will we
 	*/
-	card->parts = kmalloc_objs(struct vmupart, card->partitions);
+	card->parts = kzalloc_objs(struct vmupart, card->partitions);
 	if (!card->parts) {
 		error = -ENOMEM;
 		goto fail_partitions;
 	}
 
-	card->mtd = kmalloc_objs(struct mtd_info, card->partitions);
+	card->mtd = kzalloc_objs(struct mtd_info, card->partitions);
 	if (!card->mtd) {
 		error = -ENOMEM;
 		goto fail_mtd_info;
Signature · 1 line
2.43.0
maintainer
#3 lore raw

Re: [PATCH v2 2/3] mtd: maps: vmu-flash: fix fault in unaligned fixup

Quoted 6 lines · Florian Fuchs

Use kzalloc_obj() / kzalloc_objs() to allocate the memcard structs, instead of kmalloc_obj() / kmalloc_objs() to prevent access to uninitialized data.

Fixes runtime error: Fault in unaligned fixup: 0000 [#1] at mtd_get_fact_prot_info.

Shall this patch receive a Fixes tag if it actually fixes runtime behaviour?

Also, please add the 'Cc: stable' line in your fixes.

Thanks,
Miquèl

#4 lore raw

PATCH v2 3/3 mtd: maps: vmu-flash: Fix NULL pointer dereference in initialization

In reply to Florian Fuchs

The mtd_info contains a struct device, which must be linked to its parent. Without this, the initialization of the MTD fails with a NULL pointer dereference.

---
v1->v2: no functional change

v1: https://lore.kernel.org/lkml/[email protected]/

 drivers/mtd/maps/vmu-flash.c | 1 +
 1 file changed, 1 insertion(+)
drivers/mtd/maps/vmu-flash.c +1 -0
index b76d0609d1b7..10244e6731d0 100644
@@ -547,6 +547,7 @@ static void vmu_queryblocks(struct mapleq *mq)
 	mpart->partition = card->partition;
 	mtd_cur->priv = mpart;
 	mtd_cur->owner = THIS_MODULE;
+	mtd_cur->dev.parent = &mdev->dev;
 
 	pcache = kzalloc_obj(struct vmu_cache);
 	if (!pcache)
Signature · 1 line
2.43.0
maintainer
#5 lore raw

Re: [PATCH v2 3/3] mtd: maps: vmu-flash: Fix NULL pointer dereference in initialization

Quoted 6 lines · Florian Fuchs

The mtd_info contains a struct device, which must be linked to its parent. Without this, the initialization of the MTD fails with a NULL pointer dereference.

---

For how long as this been broken? Is it really used?

Anyway, also here you will need to Cc stable and add a fixes tag.

Thanks,
Miquèl