[U-Boot] [PATCH v3] cmd: usb_mass_storage: add protection for block_dev

[Patrick Delaunay]

Check the value of block_dev before to use this pointer.
This patch solves problem for the command "ums 0 ubi 0"
when ubifs is previously mounted; in this case the function
blk_get_device_part_str("ubi 0") don't return error
but return block_dev = NULL and then data abort.

Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
---
I check and the issue is still present on U-Boot v2018.09,
on my board stm32mp1 (with NAND device and UBI volume):

STM32MP> ubi part UBI
STM32MP> ubifsmount ubi0:boot
STM32MP> ums 0 ubi 0
data abort
pc : [<ffc60abc>]	   lr : [<ffc60ab3>]
reloc pc : [<c0110abc>]	   lr : [<c0110ab3>]
sp : fdc3e460  ip : fdc3e518	 fp : fdcf06a8
r10: fdcf06b8  r9 : fdc4eed8	 r8 : 00000000
r7 : ffce3d84  r6 : fdcf0740	 r5 : fdcf0740  r4 : ffce3d88
r3 : 00000000  r2 : 00000000	 r1 : 0000003a  r0 : 00000000
Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
Code: f04f4628 9b06fd2a bf082800 0800f04f (f5b3695b) Resetting CPU ...

Even If the command is invalid (ums not allowed on ubi device),
the data abort can be avoid by this patch.

To reproduce the issue you need to have a ubifs  already mounted,
for example with the command:
	ubi part UBI
 	ubifsmount ubi0:boot

I investigate the point, the call stack before the crash is
caused by the ubi support in
./disk/part.c =  blk_get_device_part_str()

Some part of code here are under CONFIG_CMD_UBIFS with the comment :
"ubi goes through a mtd, rathen then through a regular block device"

So the the function return a pointer to disk_partition_t :
   info->type = BOOT_PART_TYPE
   info->name =  "UBI"
but without block device information !
   *dev_desc = NULL

So the issue occurs because, when the ubifs volume is mounted,
The code in  cmd/usb_mass_storage.c

static int ums_init(const char *devtype, const char *devnums_part_str) { ...
  for (;;) {
...
     partnum = blk_get_device_part_str(devtype, devnum_part_str,
					&block_dev, &info, 1);
....
// With devnum_part_str  = "ubi 0"
// This function don't return a error and return a  valid partnum
// So the function continue until block_dev = NULL is used .....
     if (partnum < 0)
        goto cleanup;

        /* Check if the argument is in legacy format. If yes,
         * expose all partitions by setting the partnum = 0
         * e.g. ums 0 mmc 0
         */
     if (!strchr(devnum_part_str, ':'))
                 partnum = 0;

     /* f_mass_storage.c assumes SECTOR_SIZE sectors */
     if (block_dev->blksz != SECTOR_SIZE)
        goto cleanup;

=> crash occur here (on my board) because block_dev = NULL



Changes in v3:
    - Reword commit message

Changes in v2:
    - Rebase on master branch

 cmd/usb_mass_storage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmd/usb_mass_storage.c b/cmd/usb_mass_storage.c
index 0d55114..a90f7bb 100644
--- a/cmd/usb_mass_storage.c
+++ b/cmd/usb_mass_storage.c
@@ -84,7 +84,7 @@ static int ums_init(const char *devtype, const char *devnums_part_str)
 			partnum = 0;
 
 		/* f_mass_storage.c assumes SECTOR_SIZE sectors */
-		if (block_dev->blksz != SECTOR_SIZE)
+		if (!block_dev || block_dev->blksz != SECTOR_SIZE)
 			goto cleanup;
 
 		ums_new = realloc(ums, (ums_count + 1) * sizeof(*ums));
-- 
2.7.4