* [U-Boot] [PATCH] cmd: usb_mass_storage: add protection for block_dev
@ 2017-10-18 14:03 Patrick Delaunay
2017-10-18 16:27 ` Tom Rini
0 siblings, 1 reply; 5+ messages in thread
From: Patrick Delaunay @ 2017-10-18 14:03 UTC (permalink / raw)
To: u-boot
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
Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---
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 cfeecb7..c0563ca 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));
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] cmd: usb_mass_storage: add protection for block_dev
2017-10-18 14:03 [U-Boot] [PATCH] cmd: usb_mass_storage: add protection for block_dev Patrick Delaunay
@ 2017-10-18 16:27 ` Tom Rini
2017-10-18 20:22 ` Marek Vasut
0 siblings, 1 reply; 5+ messages in thread
From: Tom Rini @ 2017-10-18 16:27 UTC (permalink / raw)
To: u-boot
On Wed, Oct 18, 2017 at 04:03:21PM +0200, 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
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
> 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 cfeecb7..c0563ca 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));
Adding in Marek..
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20171018/6e56fe6f/attachment.sig>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] cmd: usb_mass_storage: add protection for block_dev
2017-10-18 16:27 ` Tom Rini
@ 2017-10-18 20:22 ` Marek Vasut
0 siblings, 0 replies; 5+ messages in thread
From: Marek Vasut @ 2017-10-18 20:22 UTC (permalink / raw)
To: u-boot
On 10/18/2017 06:27 PM, Tom Rini wrote:
> On Wed, Oct 18, 2017 at 04:03:21PM +0200, 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
>>
>> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
This is UMS, so Lukasz ... please review _thoroughly_ , as I suspect
there might be some weird underlying issue .
>> ---
>>
>> 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 cfeecb7..c0563ca 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));
>
> Adding in Marek..
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] cmd: usb_mass_storage: add protection for block_dev
2018-09-21 14:01 Patrick DELAUNAY
@ 2018-09-21 14:07 ` Marek Vasut
0 siblings, 0 replies; 5+ messages in thread
From: Marek Vasut @ 2018-09-21 14:07 UTC (permalink / raw)
To: u-boot
On 09/21/2018 04:01 PM, Patrick DELAUNAY wrote:
> Hi Marek and Lucaz
>
> I have found this old patch: See https://lists.denx.de/pipermail/u-boot/2017-October/309716.html
>
> On 10/18/2017 10:22 PM, Marek Vasut-3 wrote:
>
>> On 10/18/2017 06:27 PM, Tom Rini wrote:
>>> On Wed, Oct 18, 2017 at 04:03:21PM +0200, 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
>>>>
>>>> Signed-off-by: Patrick Delaunay <[hidden email]>
>
>> This is UMS, so Lukasz ... please review _thoroughly_ , as I suspect
>> there might be some weird underlying issue .
>
>>>> ---
>>>>
>>>> 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 cfeecb7..c0563ca 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));
>>>
>>> Adding in Marek..
>>>
>> --
>> Best regards,
>> Marek Vasut
>
> Do you think that this patch is needed.
> I check and the issue is still present on U-Boot v2018.09 :
>
> 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, I think that data abort should be avoid.
I think so too, send an updated patch please.
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] cmd: usb_mass_storage: add protection for block_dev
@ 2018-09-21 14:01 Patrick DELAUNAY
2018-09-21 14:07 ` Marek Vasut
0 siblings, 1 reply; 5+ messages in thread
From: Patrick DELAUNAY @ 2018-09-21 14:01 UTC (permalink / raw)
To: u-boot
Hi Marek and Lucaz
I have found this old patch: See https://lists.denx.de/pipermail/u-boot/2017-October/309716.html
On 10/18/2017 10:22 PM, Marek Vasut-3 wrote:
>On 10/18/2017 06:27 PM, Tom Rini wrote:
>> On Wed, Oct 18, 2017 at 04:03:21PM +0200, 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
>>>
>>> Signed-off-by: Patrick Delaunay <[hidden email]>
>This is UMS, so Lukasz ... please review _thoroughly_ , as I suspect
>there might be some weird underlying issue .
>>> ---
>>>
>>> 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 cfeecb7..c0563ca 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));
>>
>> Adding in Marek..
>>
>--
>Best regards,
>Marek Vasut
Do you think that this patch is needed.
I check and the issue is still present on U-Boot v2018.09 :
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, I think that data abort should be avoid.
Regards
Patrick
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-09-21 14:07 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-18 14:03 [U-Boot] [PATCH] cmd: usb_mass_storage: add protection for block_dev Patrick Delaunay
2017-10-18 16:27 ` Tom Rini
2017-10-18 20:22 ` Marek Vasut
2018-09-21 14:01 Patrick DELAUNAY
2018-09-21 14:07 ` Marek Vasut
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.