build icon indicating copy to clipboard operation
build copied to clipboard

[Bug]: U-Boot load address calculation bug and DT overlap oobe

Open djurny opened this issue 7 months ago • 2 comments

What happened?

Found two dormant issues after applying same solution for https://github.com/armbian/build/issues/8165 to the sunxi bootscript:

  1. An oboe in bootm_find_images() of U-Boot:v2021.04 causes false trip of overlap detection. Flattened device tree data starts at fdt_addr and ends at fdt_addr+fdt_len-1. If something else starts at fdt_addr+fdt_len it is not an overlap, it is an oobe.

    /* check if FDT overlaps OS image */
    if (images.ft_addr &&
        (((ulong)images.ft_addr >= start &&
          (ulong)images.ft_addr <= start + size) ||
         ((ulong)images.ft_addr + images.ft_len >= start &&
          (ulong)images.ft_addr + images.ft_len <= start + size))) {
    	printf("ERROR: FDT image overlaps OS image (OS=0x%lx..0x%lx)\n",
    	       start, start + size);
    	return 1;
    }
    
  2. The align_addr_next env command variable fails due to test ${modulo} -gt 0 not expecting (hex) l-value. Use itest instead to compare hex values.

How to reproduce?

Apply the same load address calculation method for boot-sunxi.cmd and test it on U-Boot v2021.04.

  1. When DT aligns exactly to the next image to load, bootm_find_images() incorrectly trips an overlap situation:

    switch to partitions #0, OK
    mmc0 is current device
    Scanning mmc 0:1...
    Found U-Boot script /boot/boot.scr
    7246 bytes read in 4 ms (1.7 MiB/s)
    ## Executing script at 43100000
    U-boot loaded from SD
    Loading environment from mmc to 0x45000000 ...
    511 bytes read in 3 ms (166 KiB/s)
    sun8i-h2-plus-orangepi-zero.dtb: No match
    Load fdt: /boot/dtb/sun8i-h2-plus-orangepi-zero.dtb
    Found mainline kernel configuration
    Loading DT from mmc to 0x43000000 ...
    35384 bytes read in 10 ms (3.4 MiB/s)
    Loading kernel provided DT overlay sun8i-h3-usbhost2.dtbo from mmc to 0x45000000 ...
    504 bytes read in 11 ms (43.9 KiB/s)
    Loading kernel provided DT overlay sun8i-h3-usbhost3.dtbo from mmc to 0x45000000 ...
    504 bytes read in 11 ms (43.9 KiB/s)
    Loading kernel provided DT overlay sun8i-h3-i2c0.dtbo from mmc to 0x45000000 ...
    374 bytes read in 10 ms (36.1 KiB/s)
    Loading user provided DT overlay rtc0-i2c-ds3231-rtc1-soc.dtbo from mmc to 0x45000000 ...
    835 bytes read in 4 ms (203.1 KiB/s)
    Loading kernel provided DT fixup script (sun8i-h3-fixup.scr) from mmc to 0x45000000 
    4185 bytes read in 11 ms (371.1 KiB/s)
    ## Executing script at 45000000
    Loading kernel from mmc to 43009000 ...
    11013984 bytes read in 838 ms (12.5 MiB/s)
    Loading ramdisk from mmc to 43a89f60 ...
    11827093 bytes read in 898 ms (12.6 MiB/s)
    Booting kernel ...
    ## Loading init Ramdisk from Legacy Image at 43a89f60 ...
       Image Name:   uInitrd
       Image Type:   ARM Linux RAMDisk Image (gzip compressed)
       Data Size:    11827029 Bytes = 11.3 MiB
       Load Address: 00000000
       Entry Point:  00000000
       Verifying Checksum ... OK
    ## Flattened Device Tree blob at 43000000
       Booting using the fdt blob at 0x43000000
    ERROR: FDT image overlaps OS image (OS=0x43009000..0x43a89f60)
    SCRIPT FAILED: continuing...
    

    DT was loaded to 43000000 and size was 0x9000 which makes DT end at 0x43008fff. Kernel was loaded to 43009000 which is not overlapping.

  2. The command variable align_next_addr fails sometimes. The test used does not handle hex values without prefix properly. test uses a simple_strtol() on both l- and r-value which borks for hexadecimal values without the 0x prefix.

    setenv align_addr_next '
    setexpr modulo ${addr_next} % ${align_to} ; 
    if test ${modulo} -gt 0 ; then 
      setexpr addr_next ${addr_next} / ${align_to} ; 
      setexpr addr_next ${addr_next} + 1 ; 
      setexpr addr_next ${addr_next} * ${align_to} ; 
    fi'
    

    (Unfolded the one-liner for easier interpretation.)

    switch to partitions #0, OK
    mmc0 is current device
    Scanning mmc 0:1...
    Found U-Boot script /boot/boot.scr
    7281 bytes read in 4 ms (1.7 MiB/s)
    ## Executing script at 43100000
    U-boot loaded from SD
    Loading environment from mmc to 0x45000000 ...
    511 bytes read in 3 ms (166 KiB/s)
    sun8i-h2-plus-orangepi-zero.dtb: No match
    Load fdt: /boot/dtb/sun8i-h2-plus-orangepi-zero.dtb
    Found mainline kernel configuration
    Loading kernel from mmc to 0x42000000 ...
    11013984 bytes read in 837 ms (12.5 MiB/s)
    Loading DT from mmc to 42a80f60 ...
    35384 bytes read in 10 ms (3.4 MiB/s)
    

    Kernel was loaded to 0x42000000 and after alignment addr_next became 42a80f60 which was not aligned to align_to at all.

Branch

main (main development branch)

On which host OS are you running the build script and observing this problem?

Ubuntu 24.04 Noble

Are you building on Windows WSL2?

  • [ ] Yes, my Ubuntu/Debian/OtherOS is running on WSL2

Relevant log URL

No response

Code of Conduct

  • [x] I agree to follow this project's Code of Conduct

djurny avatar May 11 '25 00:05 djurny

Nice debugging, thanks!

I pushed a fix, hopefully it works fine now, and without slowing down things.

ptitSeb avatar Feb 21 '25 14:02 ptitSeb

Fast turnaround! I can confirm the new version fixes the issue.

As a minor suggestion I think you can relax the check to while(bend<=end_mem), to make sure the whole address space can be used, though that somewhat depends on the behaviour of rb_get_end when there's memory in use right at the end of the space (i.e. is there a risk of it setting bend to 0 to mean 0x100000000), so perhaps the extra caution is warranted.

Another possible approach which occurs to me now would be:

do {
    // ...
} while(cur);

which I think would be sure to catch all wrap-arounds.

But anyway the fix you already have works for me - thanks!

davidje13 avatar Feb 21 '25 16:02 davidje13