[PATCH] fs: Fix reserved memory alloaction in fs_read()

Vaishnav Achath vaishnav.a at ti.com
Mon Sep 16 07:40:38 CEST 2024


The reserved memory region in fs_read() is not being
freed after read, free that. The issue was uncovered by
commit ed17a33fed29 ("lmb: make LMB memory map persistent and global")
as now the region used to load from fs cannot be reused by other
loaders like tftp, wget which use lmb_get_free_size() instead
of lmb_alloc_addr(), other loaders which uses lmb_reserve() is
appropriately freeing the buffer after usage (Eg. Serial loader).
Also while at it move #if CONFIG_IS_ENABLED(LMB) to if (IS_ENABLED()).

Fixes aa3c609e2be5 ("fs: prevent overwriting reserved memory")

Reported-by: Nishanth Menon <nm at ti.com>
Signed-off-by: Vaishnav Achath <vaishnav.a at ti.com>
---

We started seeing boot failures since we have platforms that perform
mmc load for firmwares and subsequent tftp load for kernel to same
loadaddr, the reservation need not be permanent and is only to make
sure that the read is not overwriting reserved regions, there is no
permanent reservation/protection needed for each load.

In failing case (J7200 EVM):

_snip_
run boot_rprocs
75416 bytes read in 11 ms (6.5 MiB/s)
Load Remote Processor 2 with data at addr=0x82000000 75416 bytes: Success!
75416 bytes read in 12 ms (6 MiB/s)
Load Remote Processor 3 with data at addr=0x82000000 75416 bytes: Success!
_snip_
lmb_dump_all:
 memory.count = 0x1
 memory[0]    [0x80000000-0xffffffff], 0x80000000 bytes flags: none
 reserved.count = 0x3
 reserved[0]    [0x82000000-0x82012697], 0x00012698 bytes flags: none (this is from previous mmc load, size == 12698h = 75416)
 reserved[1]    [0x9e800000-0xa47fffff], 0x06000000 bytes flags: no-map
 reserved[2]    [0xfce8fef0-0xffffffff], 0x03170110 bytes flags: no-overwrite 

With fix:
lmb_dump_all:
 memory.count = 0x1
 memory[0]    [0x80000000-0xffffffff], 0x80000000 bytes flags: none
 reserved.count = 0x2
 reserved[1]    [0x9e800000-0xa47fffff], 0x06000000 bytes flags: no-map
 reserved[2]    [0xfce8fef0-0xffffffff], 0x03170110 bytes flags: no-overwrite 

 fs/fs.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/fs/fs.c b/fs/fs.c
index 4bc28d1dff..e00c2056a5 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -552,7 +552,7 @@ static int fs_read_lmb_check(const char *filename, ulong addr, loff_t offset,
 	lmb_dump_all();
 
 	if (lmb_alloc_addr(addr, read_len) == addr)
-		return 0;
+		return read_len;
 
 	log_err("** Reading file would overwrite reserved memory **\n");
 	return -ENOSPC;
@@ -564,15 +564,13 @@ static int _fs_read(const char *filename, ulong addr, loff_t offset, loff_t len,
 {
 	struct fstype_info *info = fs_get_info(fs_type);
 	void *buf;
-	int ret;
+	int ret, alloc_size;
 
-#if CONFIG_IS_ENABLED(LMB)
-	if (do_lmb_check) {
-		ret = fs_read_lmb_check(filename, addr, offset, len, info);
-		if (ret)
-			return ret;
+	if (IS_ENABLED(CONFIG_LMB) && do_lmb_check) {
+		alloc_size = fs_read_lmb_check(filename, addr, offset, len, info);
+		if (alloc_size < 0)
+			return alloc_size;
 	}
-#endif
 
 	/*
 	 * We don't actually know how many bytes are being read, since len==0
@@ -587,6 +585,9 @@ static int _fs_read(const char *filename, ulong addr, loff_t offset, loff_t len,
 		log_debug("** %s shorter than offset + len **\n", filename);
 	fs_close();
 
+	if (IS_ENABLED(CONFIG_LMB) && do_lmb_check)
+		lmb_free(addr, alloc_size);
+
 	return ret;
 }
 
-- 
2.34.1



More information about the U-Boot mailing list