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

[Patrick DELAUNAY]

Hi Marek,

> From: Marek Vasut <marex at denx.de>
> Sent: dimanche 30 septembre 2018 10:09
> 
> On 09/28/2018 11:30 AM, Patrick DELAUNAY wrote:
> > Hi,
> 
> Hi,
> 
> >> From: Marek Vasut <marex at denx.de>
> >>
> >> On 09/26/2018 01:04 PM, Patrick Delaunay wrote:
> >>> solve data abort for the command "ums 0 ubi 0"
> >>> because result of case blk_get_device_part_str() result is OK but
> >>> with block_dev = 0 when CONFIG_CMD_UBIFS is activate and ubi volume
> >>> is mounted
> >>
> >> I don't quite understand the commit message, can you reword it?
> >
> > Ok, I prepare a V3
> 
> Sending a V3 while discussion is ongoing is pointless.

Sorry, I already sent it Friday only with more clear commit message.
Because I really ashamed by of my this fist comment... 
Even me,  I can't understand that I wrote.

> >> Also, when is this ever called with block_dev == NULL ? What's the
> >> condition to trigger this and why ?
> >
> > 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:425 =  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
> 
> Would it rather make sense to fix it here , so it has a valid dev_desc ?

I am not  confident that can be done easily,  if I correctly understood the code,  because
UBI is not really a regular block device but only directly connected to MTD to allow 
simple access to generic fs support for ubifs volume.

So NULL block devicedescriptor  is "normal" for UBIFS.

To solve the issue at this level, a fake block device should be implemented for the UBI case....
And I don't sure that I can propose something here without breaking all the other commands
using blk_get_device_part_str () function.
 
See  in ./fs/fs.c, the null dev desc is allowed for UBI

struct fstype_info fstypes[] 

#ifdef CONFIG_CMD_UBIFS
	{
		.fstype = FS_TYPE_UBIFS,
		.name = "ubifs",
		.null_dev_desc_ok = true,

> > So the issue occurs because, when the ubifs volume is mounted, The
> > code in  cmd/usb_mass_storage.c
> [...]
> 
> --
> Best regards,
> Marek Vasut

Regards

Patrick