[U-Boot] Test failures in u-boot/master on NVIDIA HW with recent push

[Stephen Warren <swarren@wwwdotorg.org>]

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) #