Re: [PATCH v11] ARM: uncompress: Validate start of physical memory against passed DTB

2020-12-12 Thread Stephen Boyd
Quoting Geert Uytterhoeven (2020-12-10 01:32:01)
> diff --git a/arch/arm/boot/compressed/fdt_check_mem_start.c 
> b/arch/arm/boot/compressed/fdt_check_mem_start.c
> new file mode 100644
> index ..e58c3a79c8a31ec4
> --- /dev/null
> +++ b/arch/arm/boot/compressed/fdt_check_mem_start.c
> @@ -0,0 +1,131 @@
[...]
> +
> +static uint64_t get_val(const fdt32_t *cells, uint32_t ncells)
> +{
> +   uint64_t r = 0;

This assignment is unnecessary?

> +
> +   r = fdt32_ld(cells);
> +   if (ncells > 1)
> +   r = (r << 32) | fdt32_ld(cells + 1);
> +
> +   return r;


[PATCH v11] ARM: uncompress: Validate start of physical memory against passed DTB

2020-12-10 Thread Geert Uytterhoeven
Currently, the start address of physical memory is obtained by masking
the program counter with a fixed mask of 0xf800.  This mask value
was chosen as a balance between the requirements of different platforms.
However, this does require that the start address of physical memory is
a multiple of 128 MiB, precluding booting Linux on platforms where this
requirement is not fulfilled.

Fix this limitation by validating the masked address against the memory
information in the passed DTB.  Only use the start address
from DTB when masking would yield an out-of-range address, prefer the
traditional method in all other cases.  Note that this applies only to the
explicitly passed DTB on modern systems, and not to a DTB appended to
the kernel, or to ATAGS.  The appended DTB may need to be augmented by
information from ATAGS, which may need to rely on knowledge of the start
address of physical memory itself.

This allows to boot Linux on r7s9210/rza2mevb using the 64 MiB of SDRAM
on the RZA2MEVB sub board, which is located at 0x0C00 (CS3 space),
i.e. not at a multiple of 128 MiB.

Suggested-by: Nicolas Pitre 
Suggested-by: Ard Biesheuvel 
Signed-off-by: Geert Uytterhoeven 
Reviewed-by: Ard Biesheuvel 
Acked-by: Nicolas Pitre 
---
I plan to submit this to Russell's patch tracker after v5.11-rc1.

v11:
  - Add Reviewed-by, Acked-by,
  - Document GOT fixup caveat,

v10:
  - Update for commit 9443076e4330a14a ("ARM: p2v: reduce p2v alignment
requirement to 2 MiB"),
  - Use OF_DT_MAGIC macro,
  - Rename fdt_get_mem_start() to fdt_check_mem_start(),
  - Skip validation if there is an appended DTB,
  - Pass mem_start to fdt_check_mem_start() to streamline code,
  - Optimize register allocation,
  - Update CONFIG_AUTO_ZRELADDR help text,
  - Check all memory nodes and ranges (not just the first one), and
"linux,usable-memory", similar to early_init_dt_scan_memory(),
  - Drop Reviewed-by, Tested-by tags,

v9:
  - Add minlen parameter to getprop(), for better validation of
memory properties,
  - Only use start of memory from the DTB if masking would yield an
out-of-range address, to fix kdump, as suggested by Ard.

v8:
  - Rebase on top of commit 893ab00439a45513 ("kbuild: remove cc-option
test of -fno-stack-protector"),

v7:
  - Rebase on top of commit 161e04a5bae58a65 ("ARM: decompressor: split
off _edata and stack base into separate object"),

v6:
  - Rebase on top of commit 7ae4a78daacf240a ("ARM: 8969/1:
decompressor: simplify libfdt builds"),
  - Include  instead of ,

v5:
  - Add Tested-by, Reviewed-by,
  - Round up start of memory to satisfy 16 MiB alignment rule,

v4:
  - Fix stack location after commit 184bf653a7a452c1 ("ARM:
decompressor: factor out routine to obtain the inflated image
size"),

v3:
  - Add Reviewed-by,
  - Fix ATAGs with appended DTB,
  - Add Tested-by,

v2:
  - Use "cmp r0, #-1", instead of "cmn r0, #1",
  - Add missing stack setup,
  - Support appended DTB.
---
 arch/arm/Kconfig  |   7 +-
 arch/arm/boot/compressed/Makefile |   5 +-
 .../arm/boot/compressed/fdt_check_mem_start.c | 131 ++
 arch/arm/boot/compressed/head.S   |  36 -
 4 files changed, 172 insertions(+), 7 deletions(-)
 create mode 100644 arch/arm/boot/compressed/fdt_check_mem_start.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 97f9d004e6d03108..cd980b070f8a99fd 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1909,9 +1909,10 @@ config AUTO_ZRELADDR
help
  ZRELADDR is the physical address where the decompressed kernel
  image will be placed. If AUTO_ZRELADDR is selected, the address
- will be determined at run-time by masking the current IP with
- 0xf800. This assumes the zImage being placed in the first 128MB
- from start of memory.
+ will be determined at run-time, either by masking the current IP
+ with 0xf800, or, if invalid, from the DTB passed in r2.
+ This assumes the zImage being placed in the first 128MB from
+ start of memory.
 
 config EFI_STUB
bool
diff --git a/arch/arm/boot/compressed/Makefile 
b/arch/arm/boot/compressed/Makefile
index fb521efcc6c20a4f..fd94e27ba4fa3d12 100644
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -87,10 +87,13 @@ libfdt_objs := fdt_rw.o fdt_ro.o fdt_wip.o fdt.o
 ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
 OBJS   += $(libfdt_objs) atags_to_fdt.o
 endif
+ifeq ($(CONFIG_USE_OF),y)
+OBJS   += $(libfdt_objs) fdt_check_mem_start.o
+endif
 
 # -fstack-protector-strong triggers protection checks in this code,
 # but it is being used too early to link to meaningful stack_chk logic.
-$(foreach o, $(libfdt_objs) atags_to_fdt.o, \
+$(foreach o, $(libfdt_objs) atags_to_fdt.o fdt_check_mem_start.o, \
$(eval CFLAGS_$(o) := -I $(srctree)/scripts/dtc/libfdt 
-fno-stack-protector))
 
 # These were previously generated C files. When you are building