[PATCH 0/1] armv8: sec_firmware: validate loadables string list

[Josh Law <josh2@disroot.org>]

Hi folks,

sec_firmware_check_copy_loadable() reads the loadables property with
fdt_getprop(), then walks it with strchr(). That works when the FIT is
well formed. If the property is malformed and the last string is missing
its trailing NUL, the walk can go past the property while looking for
the end of the entry.

The fix is to use the libfdt string list helpers for the walk. Missing
loadables still means there is nothing to copy. A malformed loadables
list now fails before any entry is used.

To check the bad case, I put a three byte loadables value with no
trailing NUL at the end of a readable page and ran the old strchr()
loop. It faults when strchr() crosses into the guard page. I also
checked the patched file still builds with:

  make O=/tmp/u-boot-sec-fw-build CROSS_COMPILE=aarch64-linux-gnu- \
       -j$(nproc) arch/arm/cpu/armv8/sec_firmware.o

I did not add the reproducer as a new test file. I couldn't find an
existing sec_firmware test harness, and adding one file for this single
case felt like churn. This is the standalone patch I used to check the
old loop. Save this as testbug.c:

  // SPDX-License-Identifier: GPL-2.0+
  #define _GNU_SOURCE
  #include <string.h>
  #include <sys/mman.h>
  #include <unistd.h>
  
  int main(void)
  {
  	const char *str;
  	long page;
  	char *area;
  	char *name;
  	int len = 3;
  
  	page = sysconf(_SC_PAGESIZE);
  	if (page <= 0)
  		return 1;
  
  	area = mmap(NULL, page * 2, PROT_READ | PROT_WRITE,
  		    MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
  	if (area == MAP_FAILED)
  		return 1;
  	if (mprotect(area + page, page, PROT_NONE))
  		return 1;
  
  	name = area + page - len;
  	memcpy(name, "tee", len);
  
  	for (str = name; str && ((str - name) < len);
  	     str = strchr(str, '\0') + 1) {
  	}
  
  	return 0;
  }

Josh Law (1):
  armv8: sec_firmware: validate loadables string list

 arch/arm/cpu/armv8/sec_firmware.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

-- 
2.47.3