Hi,
> From: Marek Vasut <ma...@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
> 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
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
> > Signed-off-by: Patrick Delaunay <patrick.delau...@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.
> >
> >
> > 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
> > 26b41b4c4..a3a338c 100644
> > --- a/cmd/usb_mass_storage.c
> > +++ b/cmd/usb_mass_storage.c
> > @@ -85,7 +85,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));
> >
>
>
> --
> Best regards,
> Marek Vasut
Regards
Patrick
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
