This fixes CVE-2018-18440 ("insufficient boundary checks in filesystem
image load") by using lmb to check the load size of a file against
reserved memory addresses.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschm...@gmail.com>
---

Changes in v2: None

 fs/fs.c       | 56 ++++++++++++++++++++++++++++++++++++++++++++++++---
 include/lmb.h |  2 ++
 lib/lmb.c     | 13 ++++++++++++
 3 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/fs/fs.c b/fs/fs.c
index adae98d021..4baf6b1c39 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -428,13 +428,57 @@ int fs_size(const char *filename, loff_t *size)
        return ret;
 }
 
-int fs_read(const char *filename, ulong addr, loff_t offset, loff_t len,
-           loff_t *actread)
+#ifdef CONFIG_LMB
+/* Check if a file may be read to the given address */
+static int fs_read_lmb_check(const char *filename, ulong addr, loff_t offset,
+                            loff_t len, struct fstype_info *info)
+{
+       struct lmb lmb;
+       int ret;
+       loff_t size;
+       loff_t read_len;
+
+       /* get the actual size of the file */
+       ret = info->size(filename, &size);
+       if (ret)
+               return ret;
+       if (offset >= size) {
+               /* offset >= EOF, no bytes will be written */
+               return 0;
+       }
+       read_len = size - offset;
+
+       /* limit to 'len' if it is smaller */
+       if (len && len < read_len)
+               read_len = len;
+
+       lmb_init_and_reserve(&lmb, gd->bd->bi_dram[0].start,
+                            gd->bd->bi_dram[0].size, (void *)gd->fdt_blob);
+       lmb_dump_all(&lmb);
+
+       if (lmb_alloc_addr(&lmb, addr, read_len) == addr)
+               return 0;
+
+       printf("** Reading file would overwrite reserved memory **\n");
+       return -1;
+}
+#endif
+
+static int _fs_read(const char *filename, ulong addr, loff_t offset, loff_t 
len,
+                   int do_lmb_check, loff_t *actread)
 {
        struct fstype_info *info = fs_get_info(fs_type);
        void *buf;
        int ret;
 
+#ifdef CONFIG_LMB
+       if (do_lmb_check) {
+               ret = fs_read_lmb_check(filename, addr, offset, len, info);
+               if (ret)
+                       return ret;
+       }
+#endif
+
        /*
         * We don't actually know how many bytes are being read, since len==0
         * means read the whole file.
@@ -451,6 +495,12 @@ int fs_read(const char *filename, ulong addr, loff_t 
offset, loff_t len,
        return ret;
 }
 
+int fs_read(const char *filename, ulong addr, loff_t offset, loff_t len,
+           loff_t *actread)
+{
+       return _fs_read(filename, addr, offset, len, 0, actread);
+}
+
 int fs_write(const char *filename, ulong addr, loff_t offset, loff_t len,
             loff_t *actwrite)
 {
@@ -621,7 +671,7 @@ int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * 
const argv[],
                pos = 0;
 
        time = get_timer(0);
-       ret = fs_read(filename, addr, pos, bytes, &len_read);
+       ret = _fs_read(filename, addr, pos, bytes, 1, &len_read);
        time = get_timer(time);
        if (ret < 0)
                return 1;
diff --git a/include/lmb.h b/include/lmb.h
index 7d7e2a78dc..62da85e716 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -31,6 +31,8 @@ struct lmb {
 extern struct lmb lmb;
 
 extern void lmb_init(struct lmb *lmb);
+extern void lmb_init_and_reserve(struct lmb *lmb, phys_addr_t base,
+                                phys_size_t size, void *fdt_blob);
 extern long lmb_add(struct lmb *lmb, phys_addr_t base, phys_size_t size);
 extern long lmb_reserve(struct lmb *lmb, phys_addr_t base, phys_size_t size);
 extern phys_addr_t lmb_alloc(struct lmb *lmb, phys_size_t size, ulong align);
diff --git a/lib/lmb.c b/lib/lmb.c
index 9de1581972..776aa7a8b7 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -104,6 +104,19 @@ void lmb_init(struct lmb *lmb)
        lmb->reserved.size = 0;
 }
 
+/* Initialize the struct, add memory and call arch/board reserve functions */
+void lmb_init_and_reserve(struct lmb *lmb, phys_addr_t base, phys_size_t size,
+                         void *fdt_blob)
+{
+       lmb_init(lmb);
+       lmb_add(lmb, base, size);
+       arch_lmb_reserve(lmb);
+       board_lmb_reserve(lmb);
+
+       if (IMAGE_ENABLE_OF_LIBFDT)
+               boot_fdt_add_mem_rsv_regions(lmb, fdt_blob);
+}
+
 /* This routine called with relocation disabled. */
 static long lmb_add_region(struct lmb_region *rgn, phys_addr_t base, 
phys_size_t size)
 {
-- 
2.17.1

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to