All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] Test failures in u-boot/master on NVIDIA HW with recent push
@ 2019-01-18  0:34 Stephen Warren
  2019-01-18  0:42 ` Tom Rini
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Warren @ 2019-01-18  0:34 UTC (permalink / raw)
  To: u-boot

Tom,

The recent set of patches pushed to u-boot/master cause DFU failures on 
both Jetson TK1 and Jetson TX1 (i.e. all platforms where I run the DFU 
test) with the following in the log:

host:
dfu-util -a 0 -U 
/var/lib/jenkins/workspace/u-boot-denx_uboot-master-test-py/U_BOOT_BOARD/jetson-tk1/build/u-boot/jetson-tk1/dfu_readback.bin 
-p 3-2.3

target:
** Reading file would overwrite reserved memory **
dfu: Read error!
dfu_read: Failed to fill buffer
Tegra124 (Jetson TK1) #

I noticed some lmb fixes in the list, so I guess it's due to that.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] Test failures in u-boot/master on NVIDIA HW with recent push
  2019-01-18  0:34 [U-Boot] Test failures in u-boot/master on NVIDIA HW with recent push Stephen Warren
@ 2019-01-18  0:42 ` Tom Rini
  2019-01-18  0:50   ` Stephen Warren
  0 siblings, 1 reply; 9+ messages in thread
From: Tom Rini @ 2019-01-18  0:42 UTC (permalink / raw)
  To: u-boot

On Thu, Jan 17, 2019 at 05:34:57PM -0700, Stephen Warren wrote:

> Tom,
> 
> The recent set of patches pushed to u-boot/master cause DFU failures on both
> Jetson TK1 and Jetson TX1 (i.e. all platforms where I run the DFU test) with
> the following in the log:
> 
> host:
> dfu-util -a 0 -U /var/lib/jenkins/workspace/u-boot-denx_uboot-master-test-py/U_BOOT_BOARD/jetson-tk1/build/u-boot/jetson-tk1/dfu_readback.bin
> -p 3-2.3
> 
> target:
> ** Reading file would overwrite reserved memory **
> dfu: Read error!
> dfu_read: Failed to fill buffer
> Tegra124 (Jetson TK1) #
> 
> I noticed some lmb fixes in the list, so I guess it's due to that.

So.. intentional!  Adding in Simon here, but I think the short answer is
that you need to change where you're saying the file goes in memory.
FWIW I run the DFU test on my dra7xx_evm and it's passing.

-- 
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/20190117/9f0ef999/attachment.sig>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] Test failures in u-boot/master on NVIDIA HW with recent push
  2019-01-18  0:42 ` Tom Rini
@ 2019-01-18  0:50   ` Stephen Warren
  2019-01-18  1:15     ` Tom Rini
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Warren @ 2019-01-18  0:50 UTC (permalink / raw)
  To: u-boot

On 1/17/19 5:42 PM, Tom Rini wrote:
> On Thu, Jan 17, 2019 at 05:34:57PM -0700, Stephen Warren wrote:
> 
>> Tom,
>>
>> The recent set of patches pushed to u-boot/master cause DFU failures on both
>> Jetson TK1 and Jetson TX1 (i.e. all platforms where I run the DFU test) with
>> the following in the log:
>>
>> host:
>> dfu-util -a 0 -U /var/lib/jenkins/workspace/u-boot-denx_uboot-master-test-py/U_BOOT_BOARD/jetson-tk1/build/u-boot/jetson-tk1/dfu_readback.bin
>> -p 3-2.3
>>
>> target:
>> ** Reading file would overwrite reserved memory **
>> dfu: Read error!
>> dfu_read: Failed to fill buffer
>> Tegra124 (Jetson TK1) #
>>
>> I noticed some lmb fixes in the list, so I guess it's due to that.
> 
> So.. intentional!  Adding in Simon here, but I think the short answer is
> that you need to change where you're saying the file goes in memory.
> FWIW I run the DFU test on my dra7xx_evm and it's passing.

You applied a change which intentionally broke functionality??? That 
sounds pretty bad...

Looking at the precise test that failed, we don't actually specify where 
the data goes in memory; it's written to the filesystem and all memmory 
locations are internally allocated by U-Boot. So when you say "you need 
to change where you're saying the file goes in memory", do you mean via 
the DFU altinfo variable (which does not specify a memory location in 
this case, so I can't), or by modifying some board-/SoC-specific config 
file or code to specify where DFU buffers data (in which case, I'd argue 
that a backwards-compatible default should have been put in place to 
prevent breaking functionality)?

The DFU altinfo values that are tested on both boards are:

Fails:

Device mmc 1 (which is an SD card):
"alt_info": "/dfu_test.bin ext4 1 1;/dfu_dummy.bin ext4 1 1",

All pass:

Device mmc 1 (which is an SD card):
"alt_info": "/dfu_test.bin part 1 3;/dfu_dummy.bin ext4 1 1",

Device mmc 1 (which is an SD card):
"alt_info": "/dfu_test.bin raw 4196352 18432;/dfu_dummy.bin ext4 1 1",

Device ram
"alt_info": "alt0 ram 80000000 01000000;alt1 ram 81000000 01000000",

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] Test failures in u-boot/master on NVIDIA HW with recent push
  2019-01-18  0:50   ` Stephen Warren
@ 2019-01-18  1:15     ` Tom Rini
  2019-01-18  6:29       ` Stephen Warren
  0 siblings, 1 reply; 9+ messages in thread
From: Tom Rini @ 2019-01-18  1:15 UTC (permalink / raw)
  To: u-boot

On Thu, Jan 17, 2019 at 05:50:27PM -0700, Stephen Warren wrote:
> On 1/17/19 5:42 PM, Tom Rini wrote:
> >On Thu, Jan 17, 2019 at 05:34:57PM -0700, Stephen Warren wrote:
> >
> >>Tom,
> >>
> >>The recent set of patches pushed to u-boot/master cause DFU failures on both
> >>Jetson TK1 and Jetson TX1 (i.e. all platforms where I run the DFU test) with
> >>the following in the log:
> >>
> >>host:
> >>dfu-util -a 0 -U /var/lib/jenkins/workspace/u-boot-denx_uboot-master-test-py/U_BOOT_BOARD/jetson-tk1/build/u-boot/jetson-tk1/dfu_readback.bin
> >>-p 3-2.3
> >>
> >>target:
> >>** Reading file would overwrite reserved memory **
> >>dfu: Read error!
> >>dfu_read: Failed to fill buffer
> >>Tegra124 (Jetson TK1) #
> >>
> >>I noticed some lmb fixes in the list, so I guess it's due to that.
> >
> >So.. intentional!  Adding in Simon here, but I think the short answer is
> >that you need to change where you're saying the file goes in memory.
> >FWIW I run the DFU test on my dra7xx_evm and it's passing.
> 
> You applied a change which intentionally broke functionality??? That sounds
> pretty bad...

So, yes.  A design decision / feature of "don't check where we're
loading payloads to" is also a security vulnerability to bypass secure
boot.  So we now have changes in that make a good attempt at keeping us
from loading a payload that can in turn overwrite ourself.  And I merged
it super early in the merge window to try and catch the unintended
consequences.

> Looking at the precise test that failed, we don't actually specify where the
> data goes in memory; it's written to the filesystem and all memmory
> locations are internally allocated by U-Boot. So when you say "you need to
> change where you're saying the file goes in memory", do you mean via the DFU
> altinfo variable (which does not specify a memory location in this case, so
> I can't), or by modifying some board-/SoC-specific config file or code to
> specify where DFU buffers data (in which case, I'd argue that a
> backwards-compatible default should have been put in place to prevent
> breaking functionality)?
> 
> The DFU altinfo values that are tested on both boards are:
> 
> Fails:
> 
> Device mmc 1 (which is an SD card):
> "alt_info": "/dfu_test.bin ext4 1 1;/dfu_dummy.bin ext4 1 1",
> 
> All pass:
> 
> Device mmc 1 (which is an SD card):
> "alt_info": "/dfu_test.bin part 1 3;/dfu_dummy.bin ext4 1 1",
> 
> Device mmc 1 (which is an SD card):
> "alt_info": "/dfu_test.bin raw 4196352 18432;/dfu_dummy.bin ext4 1 1",
> 
> Device ram
> "alt_info": "alt0 ram 80000000 01000000;alt1 ram 81000000 01000000",

So that's interesting.  How big is dfu_test.bin?  Checking my config, I
don't have SD card only RAM.  If you do RAM only tests does it pass (as
that might narrow down where maybe something is wrong) ?

-- 
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/20190117/eb4e3877/attachment.sig>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] Test failures in u-boot/master on NVIDIA HW with recent push
  2019-01-18  1:15     ` Tom Rini
@ 2019-01-18  6:29       ` Stephen Warren
  2019-01-18  8:04         ` Simon Goldschmidt
  2019-01-22  8:12         ` Simon Goldschmidt
  0 siblings, 2 replies; 9+ messages in thread
From: Stephen Warren @ 2019-01-18  6:29 UTC (permalink / raw)
  To: u-boot

On 1/17/19 6:15 PM, Tom Rini wrote:
> On Thu, Jan 17, 2019 at 05:50:27PM -0700, Stephen Warren wrote:
>> On 1/17/19 5:42 PM, Tom Rini wrote:
>>> On Thu, Jan 17, 2019 at 05:34:57PM -0700, Stephen Warren wrote:
>>>
>>>> Tom,
>>>>
>>>> The recent set of patches pushed to u-boot/master cause DFU failures on both
>>>> Jetson TK1 and Jetson TX1 (i.e. all platforms where I run the DFU test) with
>>>> the following in the log:
>>>>
>>>> host:
>>>> dfu-util -a 0 -U /var/lib/jenkins/workspace/u-boot-denx_uboot-master-test-py/U_BOOT_BOARD/jetson-tk1/build/u-boot/jetson-tk1/dfu_readback.bin
>>>> -p 3-2.3
>>>>
>>>> target:
>>>> ** Reading file would overwrite reserved memory **
>>>> dfu: Read error!
>>>> dfu_read: Failed to fill buffer
>>>> Tegra124 (Jetson TK1) #
>>>>
>>>> I noticed some lmb fixes in the list, so I guess it's due to that.
>>>
>>> So.. intentional!  Adding in Simon here, but I think the short answer is
>>> that you need to change where you're saying the file goes in memory.
>>> FWIW I run the DFU test on my dra7xx_evm and it's passing.
>>
>> You applied a change which intentionally broke functionality??? That sounds
>> pretty bad...
> 
> So, yes.  A design decision / feature of "don't check where we're
> loading payloads to" is also a security vulnerability to bypass secure
> boot.  So we now have changes in that make a good attempt at keeping us
> from loading a payload that can in turn overwrite ourself.  And I merged
> it super early in the merge window to try and catch the unintended
> consequences.
> 
>> Looking at the precise test that failed, we don't actually specify where the
>> data goes in memory; it's written to the filesystem and all memmory
>> locations are internally allocated by U-Boot. So when you say "you need to
>> change where you're saying the file goes in memory", do you mean via the DFU
>> altinfo variable (which does not specify a memory location in this case, so
>> I can't), or by modifying some board-/SoC-specific config file or code to
>> specify where DFU buffers data (in which case, I'd argue that a
>> backwards-compatible default should have been put in place to prevent
>> breaking functionality)?
>>
>> The DFU altinfo values that are tested on both boards are:
>>
>> Fails:
>>
>> Device mmc 1 (which is an SD card):
>> "alt_info": "/dfu_test.bin ext4 1 1;/dfu_dummy.bin ext4 1 1",

        "test_sizes": (
            64 - 1,
            64,
            64 + 1,
            4096 - 1,
        ),
    },

>> All pass:
>>
>> Device mmc 1 (which is an SD card):
>> "alt_info": "/dfu_test.bin part 1 3;/dfu_dummy.bin ext4 1 1",

        "test_sizes": (
            128 - 1,
            128,
            128 + 1,
            4096,
        ),

>> Device mmc 1 (which is an SD card):
>> "alt_info": "/dfu_test.bin raw 4196352 18432;/dfu_dummy.bin ext4 1 1",

        "test_sizes": (
            960 - 1,
            960,
            960 + 1,
            4096 + 1,
        ),

>> Device ram
>> "alt_info": "alt0 ram 80000000 01000000;alt1 ram 81000000 01000000",

        "test_sizes": (
            1024 * 1024 - 1,
            1024 * 1024,
            8 * 1024 * 1024,
        ),

> So that's interesting.  How big is dfu_test.bin?  Checking my config, I
> don't have SD card only RAM.  If you do RAM only tests does it pass (as
> that might narrow down where maybe something is wrong) ?

Yes, that RAM-only test passes. The tests are run in the order listed above.

test_dfu.py iterates over a bunch of different file sizes; I listed them
below the DFU configs above.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] Test failures in u-boot/master on NVIDIA HW with recent push
  2019-01-18  6:29       ` Stephen Warren
@ 2019-01-18  8:04         ` Simon Goldschmidt
  2019-01-22  8:12         ` Simon Goldschmidt
  1 sibling, 0 replies; 9+ messages in thread
From: Simon Goldschmidt @ 2019-01-18  8:04 UTC (permalink / raw)
  To: u-boot

Hi,

Am Fr., 18. Jan. 2019, 07:29 hat Stephen Warren <swarren@wwwdotorg.org>
geschrieben:

> On 1/17/19 6:15 PM, Tom Rini wrote:
> > On Thu, Jan 17, 2019 at 05:50:27PM -0700, Stephen Warren wrote:
> >> On 1/17/19 5:42 PM, Tom Rini wrote:
> >>> On Thu, Jan 17, 2019 at 05:34:57PM -0700, Stephen Warren wrote:
> >>>
> >>>> Tom,
> >>>>
> >>>> The recent set of patches pushed to u-boot/master cause DFU failures
> on both
> >>>> Jetson TK1 and Jetson TX1 (i.e. all platforms where I run the DFU
> test) with
> >>>> the following in the log:
> >>>>
> >>>> host:
> >>>> dfu-util -a 0 -U
> /var/lib/jenkins/workspace/u-boot-denx_uboot-master-test-py/U_BOOT_BOARD/jetson-tk1/build/u-boot/jetson-tk1/dfu_readback.bin
> >>>> -p 3-2.3
> >>>>
> >>>> target:
> >>>> ** Reading file would overwrite reserved memory **
> >>>> dfu: Read error!
> >>>> dfu_read: Failed to fill buffer
> >>>> Tegra124 (Jetson TK1) #
> >>>>
> >>>> I noticed some lmb fixes in the list, so I guess it's due to that.
> >>>
> >>> So.. intentional!  Adding in Simon here, but I think the short answer
> is
> >>> that you need to change where you're saying the file goes in memory.
> >>> FWIW I run the DFU test on my dra7xx_evm and it's passing.
>

I'll try to help clarifying this, but I won't have access to a PC until
Monday.

Regards,
Simon

>>
> >> You applied a change which intentionally broke functionality??? That
> sounds
> >> pretty bad...
> >
> > So, yes.  A design decision / feature of "don't check where we're
> > loading payloads to" is also a security vulnerability to bypass secure
> > boot.  So we now have changes in that make a good attempt at keeping us
> > from loading a payload that can in turn overwrite ourself.  And I merged
> > it super early in the merge window to try and catch the unintended
> > consequences.
> >
> >> Looking at the precise test that failed, we don't actually specify
> where the
> >> data goes in memory; it's written to the filesystem and all memmory
> >> locations are internally allocated by U-Boot. So when you say "you need
> to
> >> change where you're saying the file goes in memory", do you mean via
> the DFU
> >> altinfo variable (which does not specify a memory location in this
> case, so
> >> I can't), or by modifying some board-/SoC-specific config file or code
> to
> >> specify where DFU buffers data (in which case, I'd argue that a
> >> backwards-compatible default should have been put in place to prevent
> >> breaking functionality)?
> >>
> >> The DFU altinfo values that are tested on both boards are:
> >>
> >> Fails:
> >>
> >> Device mmc 1 (which is an SD card):
> >> "alt_info": "/dfu_test.bin ext4 1 1;/dfu_dummy.bin ext4 1 1",
>
>         "test_sizes": (
>             64 - 1,
>             64,
>             64 + 1,
>             4096 - 1,
>         ),
>     },
>
> >> All pass:
> >>
> >> Device mmc 1 (which is an SD card):
> >> "alt_info": "/dfu_test.bin part 1 3;/dfu_dummy.bin ext4 1 1",
>
>         "test_sizes": (
>             128 - 1,
>             128,
>             128 + 1,
>             4096,
>         ),
>
> >> Device mmc 1 (which is an SD card):
> >> "alt_info": "/dfu_test.bin raw 4196352 18432;/dfu_dummy.bin ext4 1 1",
>
>         "test_sizes": (
>             960 - 1,
>             960,
>             960 + 1,
>             4096 + 1,
>         ),
>
> >> Device ram
> >> "alt_info": "alt0 ram 80000000 01000000;alt1 ram 81000000 01000000",
>
>         "test_sizes": (
>             1024 * 1024 - 1,
>             1024 * 1024,
>             8 * 1024 * 1024,
>         ),
>
> > So that's interesting.  How big is dfu_test.bin?  Checking my config, I
> > don't have SD card only RAM.  If you do RAM only tests does it pass (as
> > that might narrow down where maybe something is wrong) ?
>
> Yes, that RAM-only test passes. The tests are run in the order listed
> above.
>
> test_dfu.py iterates over a bunch of different file sizes; I listed them
> below the DFU configs above.
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] Test failures in u-boot/master on NVIDIA HW with recent push
  2019-01-18  6:29       ` Stephen Warren
  2019-01-18  8:04         ` Simon Goldschmidt
@ 2019-01-22  8:12         ` Simon Goldschmidt
  2019-01-23 18:20           ` Stephen Warren
  1 sibling, 1 reply; 9+ messages in thread
From: Simon Goldschmidt @ 2019-01-22  8:12 UTC (permalink / raw)
  To: u-boot

Hi Stephen,
On Fri, Jan 18, 2019 at 7:29 AM Stephen Warren <swarren@wwwdotorg.org> wrote:
>
> On 1/17/19 6:15 PM, Tom Rini wrote:
> > On Thu, Jan 17, 2019 at 05:50:27PM -0700, Stephen Warren wrote:
> >> On 1/17/19 5:42 PM, Tom Rini wrote:
> >>> On Thu, Jan 17, 2019 at 05:34:57PM -0700, Stephen Warren wrote:
> >>>
> >>>> Tom,
> >>>>
> >>>> The recent set of patches pushed to u-boot/master cause DFU failures on both
> >>>> Jetson TK1 and Jetson TX1 (i.e. all platforms where I run the DFU test) with
> >>>> the following in the log:
> >>>>
> >>>> host:
> >>>> dfu-util -a 0 -U /var/lib/jenkins/workspace/u-boot-denx_uboot-master-test-py/U_BOOT_BOARD/jetson-tk1/build/u-boot/jetson-tk1/dfu_readback.bin
> >>>> -p 3-2.3
> >>>>
> >>>> target:
> >>>> ** Reading file would overwrite reserved memory **
> >>>> dfu: Read error!
> >>>> dfu_read: Failed to fill buffer
> >>>> Tegra124 (Jetson TK1) #
> >>>>
> >>>> I noticed some lmb fixes in the list, so I guess it's due to that.
> >>>
> >>> So.. intentional!  Adding in Simon here, but I think the short answer is
> >>> that you need to change where you're saying the file goes in memory.
> >>> FWIW I run the DFU test on my dra7xx_evm and it's passing.
> >>
> >> You applied a change which intentionally broke functionality??? That sounds
> >> pretty bad...
> >
> > So, yes.  A design decision / feature of "don't check where we're
> > loading payloads to" is also a security vulnerability to bypass secure
> > boot.  So we now have changes in that make a good attempt at keeping us
> > from loading a payload that can in turn overwrite ourself.  And I merged
> > it super early in the merge window to try and catch the unintended
> > consequences.
> >
> >> Looking at the precise test that failed, we don't actually specify where the
> >> data goes in memory; it's written to the filesystem and all memmory
> >> locations are internally allocated by U-Boot. So when you say "you need to
> >> change where you're saying the file goes in memory", do you mean via the DFU
> >> altinfo variable (which does not specify a memory location in this case, so
> >> I can't), or by modifying some board-/SoC-specific config file or code to
> >> specify where DFU buffers data (in which case, I'd argue that a
> >> backwards-compatible default should have been put in place to prevent
> >> breaking functionality)?
> >>
> >> The DFU altinfo values that are tested on both boards are:
> >>
> >> Fails:
> >>
> >> Device mmc 1 (which is an SD card):
> >> "alt_info": "/dfu_test.bin ext4 1 1;/dfu_dummy.bin ext4 1 1",
>
>         "test_sizes": (
>             64 - 1,
>             64,
>             64 + 1,
>             4096 - 1,
>         ),
>     },
>
> >> All pass:
> >>
> >> Device mmc 1 (which is an SD card):
> >> "alt_info": "/dfu_test.bin part 1 3;/dfu_dummy.bin ext4 1 1",
>
>         "test_sizes": (
>             128 - 1,
>             128,
>             128 + 1,
>             4096,
>         ),
>
> >> Device mmc 1 (which is an SD card):
> >> "alt_info": "/dfu_test.bin raw 4196352 18432;/dfu_dummy.bin ext4 1 1",
>
>         "test_sizes": (
>             960 - 1,
>             960,
>             960 + 1,
>             4096 + 1,
>         ),
>
> >> Device ram
> >> "alt_info": "alt0 ram 80000000 01000000;alt1 ram 81000000 01000000",
>
>         "test_sizes": (
>             1024 * 1024 - 1,
>             1024 * 1024,
>             8 * 1024 * 1024,
>         ),
>
> > So that's interesting.  How big is dfu_test.bin?  Checking my config, I
> > don't have SD card only RAM.  If you do RAM only tests does it pass (as
> > that might narrow down where maybe something is wrong) ?
>
> Yes, that RAM-only test passes. The tests are run in the order listed above.
>
> test_dfu.py iterates over a bunch of different file sizes; I listed them
> below the DFU configs above.

OK, I have absolutely no experience with DFU, so please be patient with me
when interpreting this wrong...

Is it correct that the files above differ in that the one failing is
read via the ext4 fs
driver? In that case, I guess it might be the ext4 fs driver trying to
load something
into a buffer on the stack?

Could you try the attached patch where I added more debugging output? Maybe
I can read something from its output.

Thanks,
Simon
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lmb-debug-dfu-failure.patch
Type: application/octet-stream
Size: 1435 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190122/76967266/attachment.obj>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] Test failures in u-boot/master on NVIDIA HW with recent push
  2019-01-22  8:12         ` Simon Goldschmidt
@ 2019-01-23 18:20           ` Stephen Warren
  2019-01-23 20:39             ` Simon Goldschmidt
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Warren @ 2019-01-23 18:20 UTC (permalink / raw)
  To: u-boot

On 1/22/19 1:12 AM, Simon Goldschmidt wrote:
> Hi Stephen,
> On Fri, Jan 18, 2019 at 7:29 AM Stephen Warren <swarren@wwwdotorg.org> wrote:
>>
>> On 1/17/19 6:15 PM, Tom Rini wrote:
>>> On Thu, Jan 17, 2019 at 05:50:27PM -0700, Stephen Warren wrote:
>>>> On 1/17/19 5:42 PM, Tom Rini wrote:
>>>>> On Thu, Jan 17, 2019 at 05:34:57PM -0700, Stephen Warren wrote:
>>>>>
>>>>>> Tom,
>>>>>>
>>>>>> The recent set of patches pushed to u-boot/master cause DFU failures on both
>>>>>> Jetson TK1 and Jetson TX1 (i.e. all platforms where I run the DFU test) with
>>>>>> the following in the log:
>>>>>>
>>>>>> host:
>>>>>> dfu-util -a 0 -U /var/lib/jenkins/workspace/u-boot-denx_uboot-master-test-py/U_BOOT_BOARD/jetson-tk1/build/u-boot/jetson-tk1/dfu_readback.bin
>>>>>> -p 3-2.3
>>>>>>
>>>>>> target:
>>>>>> ** Reading file would overwrite reserved memory **
>>>>>> dfu: Read error!
>>>>>> dfu_read: Failed to fill buffer
>>>>>> Tegra124 (Jetson TK1) #
>>>>>>
>>>>>> I noticed some lmb fixes in the list, so I guess it's due to that.
>>>>>
>>>>> So.. intentional!  Adding in Simon here, but I think the short answer is
>>>>> that you need to change where you're saying the file goes in memory.
>>>>> FWIW I run the DFU test on my dra7xx_evm and it's passing.
>>>>
>>>> You applied a change which intentionally broke functionality??? That sounds
>>>> pretty bad...
>>>
>>> So, yes.  A design decision / feature of "don't check where we're
>>> loading payloads to" is also a security vulnerability to bypass secure
>>> boot.  So we now have changes in that make a good attempt at keeping us
>>> from loading a payload that can in turn overwrite ourself.  And I merged
>>> it super early in the merge window to try and catch the unintended
>>> consequences.
>>>
>>>> Looking at the precise test that failed, we don't actually specify where the
>>>> data goes in memory; it's written to the filesystem and all memmory
>>>> locations are internally allocated by U-Boot. So when you say "you need to
>>>> change where you're saying the file goes in memory", do you mean via the DFU
>>>> altinfo variable (which does not specify a memory location in this case, so
>>>> I can't), or by modifying some board-/SoC-specific config file or code to
>>>> specify where DFU buffers data (in which case, I'd argue that a
>>>> backwards-compatible default should have been put in place to prevent
>>>> breaking functionality)?
>>>>
>>>> The DFU altinfo values that are tested on both boards are:
>>>>
>>>> Fails:
>>>>
>>>> Device mmc 1 (which is an SD card):
>>>> "alt_info": "/dfu_test.bin ext4 1 1;/dfu_dummy.bin ext4 1 1",
>>
>>          "test_sizes": (
>>              64 - 1,
>>              64,
>>              64 + 1,
>>              4096 - 1,
>>          ),
>>      },
>>
>>>> All pass:
>>>>
>>>> Device mmc 1 (which is an SD card):
>>>> "alt_info": "/dfu_test.bin part 1 3;/dfu_dummy.bin ext4 1 1",
>>
>>          "test_sizes": (
>>              128 - 1,
>>              128,
>>              128 + 1,
>>              4096,
>>          ),
>>
>>>> Device mmc 1 (which is an SD card):
>>>> "alt_info": "/dfu_test.bin raw 4196352 18432;/dfu_dummy.bin ext4 1 1",
>>
>>          "test_sizes": (
>>              960 - 1,
>>              960,
>>              960 + 1,
>>              4096 + 1,
>>          ),
>>
>>>> Device ram
>>>> "alt_info": "alt0 ram 80000000 01000000;alt1 ram 81000000 01000000",
>>
>>          "test_sizes": (
>>              1024 * 1024 - 1,
>>              1024 * 1024,
>>              8 * 1024 * 1024,
>>          ),
>>
>>> So that's interesting.  How big is dfu_test.bin?  Checking my config, I
>>> don't have SD card only RAM.  If you do RAM only tests does it pass (as
>>> that might narrow down where maybe something is wrong) ?
>>
>> Yes, that RAM-only test passes. The tests are run in the order listed above.
>>
>> test_dfu.py iterates over a bunch of different file sizes; I listed them
>> below the DFU configs above.
> 
> OK, I have absolutely no experience with DFU, so please be patient with me
> when interpreting this wrong...
> 
> Is it correct that the files above differ in that the one failing is
> read via the ext4 fs

Yes.

Note that not all reads via ext4 fail, just in the case where *both* of 
the two DFU-accessible storage regions are on ext4.

> driver? In that case, I guess it might be the ext4 fs driver trying to
> load something into a buffer on the stack?

I don't know the ext4 code well enough, but in general yes it might be 
using stack rather than malloc/similar buffers.

> Could you try the attached patch where I added more debugging output? Maybe
> I can read something from its output.

Target:

setenv "dfu_alt_info" "/dfu_test.bin ext4 1 1;/dfu_dummy.bin ext4 1 1"

dfu 0 mmc 1

Host:

dfu-util -a 0 -D 
/home/swarren/shared/git_wa/tegra-uboot-flasher/u-boot/build-p2371-2180/persistent-data/dfu_dummy.bin 
-p 3-13

Target:

mmc_file_op: ext4write mmc 1:1 00000000dda3eb40 /dfu_test.bin 400 
0x00000000dda26210

Host:

dfu-util -a 0 -D 
/home/swarren/shared/git_wa/tegra-uboot-flasher/u-boot/build-p2371-2180/persistent-data/dfu_63.bin 
-p 3-13

Target:

mmc_file_op: ext4write mmc 1:1 00000000dda3eb40 /dfu_test.bin 3f 
0x00000000dda26210

Host:

dfu-util -a 1 -D 
/home/swarren/shared/git_wa/tegra-uboot-flasher/u-boot/build-p2371-2180/persistent-data/dfu_dummy.bin 
-p 3-13

Target:

mmc_file_op: ext4write mmc 1:1 00000000dda3eb40 /dfu_dummy.bin 400 
0x00000000dda26210

Host:

dfu-util -a 0 -U 
/home/swarren/shared/git_wa/tegra-uboot-flasher/u-boot/build-p2371-2180/dfu_readback.bin 
-p 3-13

Target:

mmc_file_op: ext4size mmc 1:1 /dfu_test.bin 0x00000000dda260c0
mmc_file_op: ext4load mmc 1:1 00000000dda3eb40 /dfu_test.bin 
0x00000000dda260c0
lmb_dump_all:
     memory.cnt		   = 0x1
     memory.size		   = 0x0
     memory.reg[0x0].base   = 0x80000000
		   .size   = 0x60000000

     reserved.cnt	   = 0x1
     reserved.size	   = 0x0
     reserved.reg[0x0].base = 0xdda24c50
		     .size = 0x25db3b0
** Reading file would overwrite reserved memory (addr: dda3eb40; len: 3f)**
dfu: Read error!
dfu_read: Failed to fill buffer
Tegra210 (P2371-2180) #

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] Test failures in u-boot/master on NVIDIA HW with recent push
  2019-01-23 18:20           ` Stephen Warren
@ 2019-01-23 20:39             ` Simon Goldschmidt
  0 siblings, 0 replies; 9+ messages in thread
From: Simon Goldschmidt @ 2019-01-23 20:39 UTC (permalink / raw)
  To: u-boot

Am Mi., 23. Jan. 2019, 19:20 hat Stephen Warren <swarren@wwwdotorg.org>
geschrieben:

> On 1/22/19 1:12 AM, Simon Goldschmidt wrote:
> > Hi Stephen,
> > On Fri, Jan 18, 2019 at 7:29 AM Stephen Warren <swarren@wwwdotorg.org>
> wrote:
> >>
> >> On 1/17/19 6:15 PM, Tom Rini wrote:
> >>> On Thu, Jan 17, 2019 at 05:50:27PM -0700, Stephen Warren wrote:
> >>>> On 1/17/19 5:42 PM, Tom Rini wrote:
> >>>>> On Thu, Jan 17, 2019 at 05:34:57PM -0700, Stephen Warren wrote:
> >>>>>
> >>>>>> Tom,
> >>>>>>
> >>>>>> The recent set of patches pushed to u-boot/master cause DFU
> failures on both
> >>>>>> Jetson TK1 and Jetson TX1 (i.e. all platforms where I run the DFU
> test) with
> >>>>>> the following in the log:
> >>>>>>
> >>>>>> host:
> >>>>>> dfu-util -a 0 -U
> /var/lib/jenkins/workspace/u-boot-denx_uboot-master-test-py/U_BOOT_BOARD/jetson-tk1/build/u-boot/jetson-tk1/dfu_readback.bin
> >>>>>> -p 3-2.3
> >>>>>>
> >>>>>> target:
> >>>>>> ** Reading file would overwrite reserved memory **
> >>>>>> dfu: Read error!
> >>>>>> dfu_read: Failed to fill buffer
> >>>>>> Tegra124 (Jetson TK1) #
> >>>>>>
> >>>>>> I noticed some lmb fixes in the list, so I guess it's due to that.
> >>>>>
> >>>>> So.. intentional!  Adding in Simon here, but I think the short
> answer is
> >>>>> that you need to change where you're saying the file goes in memory.
> >>>>> FWIW I run the DFU test on my dra7xx_evm and it's passing.
> >>>>
> >>>> You applied a change which intentionally broke functionality??? That
> sounds
> >>>> pretty bad...
> >>>
> >>> So, yes.  A design decision / feature of "don't check where we're
> >>> loading payloads to" is also a security vulnerability to bypass secure
> >>> boot.  So we now have changes in that make a good attempt at keeping us
> >>> from loading a payload that can in turn overwrite ourself.  And I
> merged
> >>> it super early in the merge window to try and catch the unintended
> >>> consequences.
> >>>
> >>>> Looking at the precise test that failed, we don't actually specify
> where the
> >>>> data goes in memory; it's written to the filesystem and all memmory
> >>>> locations are internally allocated by U-Boot. So when you say "you
> need to
> >>>> change where you're saying the file goes in memory", do you mean via
> the DFU
> >>>> altinfo variable (which does not specify a memory location in this
> case, so
> >>>> I can't), or by modifying some board-/SoC-specific config file or
> code to
> >>>> specify where DFU buffers data (in which case, I'd argue that a
> >>>> backwards-compatible default should have been put in place to prevent
> >>>> breaking functionality)?
> >>>>
> >>>> The DFU altinfo values that are tested on both boards are:
> >>>>
> >>>> Fails:
> >>>>
> >>>> Device mmc 1 (which is an SD card):
> >>>> "alt_info": "/dfu_test.bin ext4 1 1;/dfu_dummy.bin ext4 1 1",
> >>
> >>          "test_sizes": (
> >>              64 - 1,
> >>              64,
> >>              64 + 1,
> >>              4096 - 1,
> >>          ),
> >>      },
> >>
> >>>> All pass:
> >>>>
> >>>> Device mmc 1 (which is an SD card):
> >>>> "alt_info": "/dfu_test.bin part 1 3;/dfu_dummy.bin ext4 1 1",
> >>
> >>          "test_sizes": (
> >>              128 - 1,
> >>              128,
> >>              128 + 1,
> >>              4096,
> >>          ),
> >>
> >>>> Device mmc 1 (which is an SD card):
> >>>> "alt_info": "/dfu_test.bin raw 4196352 18432;/dfu_dummy.bin ext4 1 1",
> >>
> >>          "test_sizes": (
> >>              960 - 1,
> >>              960,
> >>              960 + 1,
> >>              4096 + 1,
> >>          ),
> >>
> >>>> Device ram
> >>>> "alt_info": "alt0 ram 80000000 01000000;alt1 ram 81000000 01000000",
> >>
> >>          "test_sizes": (
> >>              1024 * 1024 - 1,
> >>              1024 * 1024,
> >>              8 * 1024 * 1024,
> >>          ),
> >>
> >>> So that's interesting.  How big is dfu_test.bin?  Checking my config, I
> >>> don't have SD card only RAM.  If you do RAM only tests does it pass (as
> >>> that might narrow down where maybe something is wrong) ?
> >>
> >> Yes, that RAM-only test passes. The tests are run in the order listed
> above.
> >>
> >> test_dfu.py iterates over a bunch of different file sizes; I listed them
> >> below the DFU configs above.
> >
> > OK, I have absolutely no experience with DFU, so please be patient with
> me
> > when interpreting this wrong...
> >
> > Is it correct that the files above differ in that the one failing is
> > read via the ext4 fs
>
> Yes.
>
> Note that not all reads via ext4 fail, just in the case where *both* of
> the two DFU-accessible storage regions are on ext4.
>

Yeah, I didn't really get it why the other tests work...


> > driver? In that case, I guess it might be the ext4 fs driver trying to
> > load something into a buffer on the stack?
>
> I don't know the ext4 code well enough, but in general yes it might be
> using stack rather than malloc/similar buffers.
>

Checking that again, it's the buffer allocated by dfu via malloc that
doesn't work and it has nothing to do with ext4.

The memory returned by malloc is of course in the protected range. The
protection is written with the CLI in mind where the user passes an address
via variables. When using malloced buffers like DFU does, it should work if
we call the correct function (like fs_read) instead of taking the long way
via run_command().

I'll see if I can provide a patch for that soon.

Regards,
Simon


> > Could you try the attached patch where I added more debugging output?
> Maybe
> > I can read something from its output.
>
> Target:
>
> setenv "dfu_alt_info" "/dfu_test.bin ext4 1 1;/dfu_dummy.bin ext4 1 1"
>
> dfu 0 mmc 1
>
> Host:
>
> dfu-util -a 0 -D
> /home/swarren/shared/git_wa/tegra-uboot-flasher/u-boot/build-p2371-2180/persistent-data/dfu_dummy.bin
>
> -p 3-13
>
> Target:
>
> mmc_file_op: ext4write mmc 1:1 00000000dda3eb40 /dfu_test.bin 400
> 0x00000000dda26210
>
> Host:
>
> dfu-util -a 0 -D
> /home/swarren/shared/git_wa/tegra-uboot-flasher/u-boot/build-p2371-2180/persistent-data/dfu_63.bin
>
> -p 3-13
>
> Target:
>
> mmc_file_op: ext4write mmc 1:1 00000000dda3eb40 /dfu_test.bin 3f
> 0x00000000dda26210
>
> Host:
>
> dfu-util -a 1 -D
> /home/swarren/shared/git_wa/tegra-uboot-flasher/u-boot/build-p2371-2180/persistent-data/dfu_dummy.bin
>
> -p 3-13
>
> Target:
>
> mmc_file_op: ext4write mmc 1:1 00000000dda3eb40 /dfu_dummy.bin 400
> 0x00000000dda26210
>
> Host:
>
> dfu-util -a 0 -U
> /home/swarren/shared/git_wa/tegra-uboot-flasher/u-boot/build-p2371-2180/dfu_readback.bin
>
> -p 3-13
>
> Target:
>
> mmc_file_op: ext4size mmc 1:1 /dfu_test.bin 0x00000000dda260c0
> mmc_file_op: ext4load mmc 1:1 00000000dda3eb40 /dfu_test.bin
> 0x00000000dda260c0
> lmb_dump_all:
>      memory.cnt            = 0x1
>      memory.size                   = 0x0
>      memory.reg[0x0].base   = 0x80000000
>                    .size   = 0x60000000
>
>      reserved.cnt          = 0x1
>      reserved.size         = 0x0
>      reserved.reg[0x0].base = 0xdda24c50
>                      .size = 0x25db3b0
> ** Reading file would overwrite reserved memory (addr: dda3eb40; len: 3f)**
> dfu: Read error!
> dfu_read: Failed to fill buffer
> Tegra210 (P2371-2180) #
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-01-23 20:39 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-18  0:34 [U-Boot] Test failures in u-boot/master on NVIDIA HW with recent push Stephen Warren
2019-01-18  0:42 ` Tom Rini
2019-01-18  0:50   ` Stephen Warren
2019-01-18  1:15     ` Tom Rini
2019-01-18  6:29       ` Stephen Warren
2019-01-18  8:04         ` Simon Goldschmidt
2019-01-22  8:12         ` Simon Goldschmidt
2019-01-23 18:20           ` Stephen Warren
2019-01-23 20:39             ` Simon Goldschmidt

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.