Hi Evgeny, On Fri, 29 Nov 2024 at 03:03, Evgeny Bachinin <[email protected]> wrote: > > Hello, dear reviewers! > > It was my 1st attempt to use b4-tool and it did not list all relevant > reviewers. > > So, let me manually CC relevant stakeholders/maintainers. > Please, take a look on the fix of bug/vulnerability > > P.S. If you wish, advise me, please, whether I need to re-send with > updated CC list or just publish v2. > > On Mon, Nov 25, 2024 at 12:15:07PM +0300, Evgeny Bachinin wrote: > > Patch resolves two kind of bugs, one of which is vulnerability related > > to KASLR. > > > > === Issue briefly === > > > > Working with FDT (via non-relocated gd::fdt_blob) from inside bootm > > command may lead to the reading the garbage instead of FDT nodes. And > > this can result in various side-effects depending on DTS nodes, being > > parsed during bootm. > > > > But below is my specific story how I faced with this issue due to > > MESON_RNG probing failure. > > > > === Bugs description === > > > > 1) Bug is revealed on: > > * configuration below > > * U-boot 2024.10 - f919c3a889f ("Prepare v2024.10") > > > > It seems, the following patch is a trigger: > > ea955eea4f ("fdt: automatically add /chosen/kaslr-seed if DM_RNG is > > enabled") > > > > Generally, CONFIG_OF_EMBED=y & CONFIG_RNG_MESON=y are the most > > valuable ones for reproducing the issue. > > ``` > > CONFIG_ARCH_FIXUP_FDT_MEMORY=y > > CONFIG_CMD_FDT=y > > CONFIG_DEFAULT_FDT_FILE="" > > CONFIG_FDT_64BIT=y > > CONFIG_OF_BOARD_SETUP=y > > CONFIG_OF_CONTROL=y > > CONFIG_OF_EMBED=y > > CONFIG_OF_LIBFDT_ASSUME_MASK=0x0 > > CONFIG_OF_LIBFDT_OVERLAY=y > > CONFIG_OF_LIBFDT=y > > CONFIG_OF_LIST="meson-axg-our-device-name" > > CONFIG_OF_REAL=y > > CONFIG_OF_TRANSLATE=y > > CONFIG_SUPPORT_OF_CONTROL=y > > CONFIG_SYS_FDT_PAD=0x3000 > > CONFIG_TOOLS_OF_LIBFDT=y > > > > CONFIG_DM_RNG=y > > CONFIG_RNG_MESON=y > > ``` > > > > 2) Due to CONFIG_OF_EMBED, the DTS is embedded into U-boot ELF and > > accessible via __dtb_dt_begin symbol. > > > > On early boot stage (board_f.c) the fdtdec_setup() is called only > > once before U-boot's relocation into top of RAM. fdtdec_setup() > > initializes gd::fdt_blob for FDTSRC_EMBED case: > > ``` > > gd->fdt_blob = dtb_dt_embedded(); > > gd->fdt_src = FDTSRC_EMBED; > > ``` > > > > 3) Then reloc_fdt() is called in board_f.c > > > > But due to CONFIG_OF_EMBED=y the reloc_fdt() does not update > > gd::fdt_blob value (strictly speaking, it is impossible for > > CONFIG_OF_EMBED=y, because U-boot ELF has not been relocated yet > > at this moment). > > > > As a result after relocation we get fdt_blob, pointing to DTS address > > before relocation: > > ``` > > # bdinfo > > <...> > > relocaddr = 0x000000000fedf000 > > reloc off = 0x000000000eedf000 > > <...> > > fdt_blob = 0x010ce6c0 << points to __dtb_dt_begin before relocation > > new_fdt = 0x0000000000000000 << empty erroneously > > fdt_size = 0x0000000000000000 << zero erroneously > > ``` > > > > 4) During bootm command (according to our ITS-config file) the Linux > > is loaded into 0x01080000 (which is very close to fdt_blob addr > > 0x010ce6c0). > > ``` > > ## Loading kernel from FIT Image at 04000000 ... > > Trying 'kernel' kernel subimage > > <...> > > Load Address: 0x01080000 > > ``` > > > > So Linux image overwrites the gd::fdt_blob memory location > > in RAM (0x010ce6c0). > > > > 5) Issue: > > > > Hence any manipulation with DTS (say, via FDT API) inside > > implementation of bootm command leads to accessing the fdt_blob area > > with garbage, that can lead to two situations: > > > > 5.1) Abort. > > > > Call to fdt_off_dt_struct() from fdt_next_tag() :: fdt_offset_ptr():: > > fdt_offset_ptr_() returns with garbage, that leads to tagp value > > being out of RAM top addr (256 Mb in our board), causing the abort: > > ``` > > Boot cmd: bootm 0x4000000#boot_evt1 > > bootm_run_states() > > <...> > > image_setup_libfdt() > > fdt_chosen() > > fdt_kaslrseed() > > uclass_get_device() > > uclass_get_device_tail() > > device_probe() > > device_of_to_plat() > > meson_rng_of_to_plat() > > clk_get_by_name_optional() > > clk_get_by_name() > > clk_get_by_name_nodev() > > ofnode_stringlist_search() > > fdt_stringlist_search() > > fdt_getprop() > > fdt_get_property_namelen_() > > fdt_first_property_offset() > > fdt_check_node_offset_() > > fdt_next_tag(): > > ``` > > tagp = fdt_offset_ptr(fdt, offset, FDT_TAGSIZE); > > ``` > > fdt_next_tag() tagp:0x22890766 > > fdt_next_tag() ram_top:0x10000000 (tagp OUT of RAM) > > "Synchronous Abort" handler, esr 0x96000010, far 0x22890766 > > elr: 000000000108be24 lr : 000000000108be24 (reloc) > > elr: 000000000ff6fe24 lr : 000000000ff6fe24 > > x0 : 0000000000000041 x1 : 0000000000000000 > > x2 : 000000000ff3b57c x3 : 0000000000000012 > > x4 : 000000000ded2ad5 x5 : 0000000000000020 > > x6 : 00000000ffffffe8 x7 : 000000000ded2f40 > > x8 : 00000000ffffffd8 x9 : 000000000000000d > > x10: 0000000000000006 x11: 000000000001869f > > x12: 000000000fffffff x13: 000000000fffffff > > x14: 0000000000000000 x15: 000000000ded2abb > > x16: 000000000ff3b080 x17: 0000000000000001 > > x18: 000000000ded3dc0 x19: 0000000022890766 > > x20: 00000000010cb0f0 x21: 00000000000015e4 > > x22: 000000000ff8f4d8 x23: 000000000000000b > > x24: 000000000ded2fbc x25: 000000000ffe2000 > > x22: 000000000ff8f4d8 x23: 000000000000000b > > x24: 000000000ded2fbc x25: 000000000ffe2000 > > x26: 000000000ffe2000 x27: 000000000000000b > > x28: 000000000ff9cf2d x29: 000000000ded2f40 > > > > Code: aa1603e1 91197484 52801742 94004de8 (b9400276) > > ``` > > > > 5.2) Vulnerability situation "KASLR is disabled". > > > > Almost the same as in (5.1), but 2 situations happen (depending on > > the value of garbage): > > * call to fdt_offset_ptr_() :: fdt_off_dt_struct(fdt) > > returns not so big garbage, leading to tagp, being inside RAM. > > * or calculations of absoffset inside fdt_offset_ptr() leads to > > failure of the one of if() conditions with NULL as retval. > > > > Result is fdt_next_tag() interprets the tagp as FDT_END. And we are > > returning from our callstack via functions' error paths, leading to > > "No RNG device" and "KASLR disabled due to lack of seed": > > ``` > > fdt_kaslrseed() > > uclass_get_device() > > <...> > > device_probe() > > device_of_to_plat() > > meson_rng_of_to_plat() > > clk_get_by_name() > > clk_get_by_name_nodev() > > <...> > > fdt_stringlist_search() > > fdt_getprop() > > fdt_get_property_namelen_() > > fdt_first_property_offset() > > fdt_check_node_offset_() > > fdt_next_tag(): > > ``` > > tagp = fdt_offset_ptr(fdt, offset, FDT_TAGSIZE); > > ``` > > fdt_next_tag() tagp:0000000001890677 > > fdt_next_tag() ram_top:0x10000000 (tagp is inside RAM) > > uclass_get_device_tail():486 device_probe() ret:-22 > > No RNG device > > Starting kernel ... > > > > [ 0.000000] Linux version 6.9.12 > > [ 0.000000] KASLR disabled due to lack of seed > > ``` > > > > Signed-off-by: Evgeny Bachinin <[email protected]> > > --- > > Tested: > > * on board with Amlogic AXG SoC. > > * on sandbox: make sure that certain code from patch is called (of > > course, when CONFIG_OF_EMBED=y) > > * U-boot CI: https://github.com/u-boot/u-boot/pull/698/checks > > --- > > common/board_r.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/common/board_r.c b/common/board_r.c > > index > > 62228a723e12f8742437acae380e82f79903a8f5..88dc756b2a5e5a44f55f9b0fd012adc798a8afdb > > 100644 > > --- a/common/board_r.c > > +++ b/common/board_r.c > > @@ -152,6 +152,15 @@ static int initr_reloc_global_data(void) > > */ > > gd->env_addr += gd->reloc_off; > > #endif > > + > > + /* > > + * For CONFIG_OF_EMBED case the FDT is embedded into ELF, available by > > + * __dtb_dt_begin. After U-boot ELF self-relocation to RAM top address
U-Boot > > + * it is worth to update fdt_blob in global_data > > + */ > > + if (IS_ENABLED(CONFIG_OF_EMBED)) > > + gd->fdt_blob = dtb_dt_embedded(); > > + > > #ifdef CONFIG_EFI_LOADER > > /* > > * On the ARM architecture gd is mapped to a fixed register (r9 or > > x18). > > > > --- > > base-commit: cca05617a8f585f3a98a8fa82f75cc68a530d771 > > change-id: 20241119-of_embed_fdt_blob_garbage-ea729a8236e1 > > > > Best regards, > > -- > > Evgeny Bachinin <[email protected]> Note that OF_EMBED is a development feature[1], for debugging with JTAG, etc. Which board is this? Also, you should normally disable relocation (with gd->flags GD_FLG_SKIP_RELOC) when using OF_EMBED, since the code ends up elsewhere, which is a pain when debugging. This macro is for use by fdtgrep...perhaps I should have added an underscore at the end to make that clear. So please add a function in fdtdec to handle this, calling it from initr_reloc_global_data(), perhaps. Regards, Simon [1] https://docs.u-boot.org/en/latest/develop/devicetree/control.html#configuration
