fs: prevent overwriting reserved memory

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.goldschmidt@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>

diff --git a/fs/fs.c b/fs/fs.c
index cb265174e2f2eb446bd6003903cb1e1408aa6f4e..7fd22101efe657b72b48c472a4d9dd100c4aedc9 100644 (file)
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -429,13 +429,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 -ENOSPC;
+}
+#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.
@@ -452,6 +496,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)
 {
@@ -622,7 +672,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 7d7e2a78dc0be741351f9aab176f14282273d9c1..62da85e716e1b73ef5b318b6ca875eb69106fe2d 100644 (file)
--- 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 e380a0a722fe5e966660b6c2b7b67604b1daeb84..3407705fa7030029baf97d03cf5beec1eb9187c5 100644 (file)
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -98,6 +98,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 && fdt_blob)
+               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)
 {