| Message ID | 20190126211304.17356-1-simon.k.r.goldschmidt@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot] lmb: handle more than one DRAM BANK | expand |
On 1/26/19 10:13 PM, Simon Goldschmidt wrote: > This fixes the automatic lmb initialization and reservation for boards > with more than one DRAM bank. > > This fixes the CVE-2018-18439 and -18440 fixes that only allowed to load > files into the firs DRAM bank from fs and via tftp. > > Found-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> Thanks for the patch. With the patch tftp to $loadaddr is possible on vexpress_ca15_tc2. Loading to an address outside DRAM is forbidden. So for vexpress_ca15_tc2 Tested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
On 1/26/19 10:27 PM, Heinrich Schuchardt wrote: > On 1/26/19 10:13 PM, Simon Goldschmidt wrote: >> This fixes the automatic lmb initialization and reservation for boards >> with more than one DRAM bank. >> >> This fixes the CVE-2018-18439 and -18440 fixes that only allowed to load >> files into the firs DRAM bank from fs and via tftp. >> >> Found-by: Heinrich Schuchardt <xypron.glpk@gmx.de> >> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > > Thanks for the patch. > > With the patch tftp to $loadaddr is possible on vexpress_ca15_tc2. > Loading to an address outside DRAM is forbidden. > > So for vexpress_ca15_tc2 > > Tested-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > I ran this patch through Travis CI together with the efi-next queue and no error occurred. https://travis-ci.org/xypron2/u-boot/builds/484859383 https://github.com/xypron2/u-boot/commits/21ea2188022313cac9db0f27d46a9dbcad0f2c19 Best regards Heinrich
Hi Simon, On Sat, 26 Jan 2019 at 14:13, Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> wrote: > > This fixes the automatic lmb initialization and reservation for boards > with more than one DRAM bank. > > This fixes the CVE-2018-18439 and -18440 fixes that only allowed to load > files into the firs DRAM bank from fs and via tftp. > > Found-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > --- > > common/bootm.c | 4 ++-- > fs/fs.c | 3 +-- > include/lmb.h | 7 +++++-- > lib/lmb.c | 37 ++++++++++++++++++++++++++++++++----- > net/tftp.c | 3 +-- > 5 files changed, 41 insertions(+), 13 deletions(-) Reviewed-by: Simon Glass <sjg@chromium.org> Do we need a test update to cover this? - Simon
On 1/31/19 11:04 AM, Simon Glass wrote: > Hi Simon, > > On Sat, 26 Jan 2019 at 14:13, Simon Goldschmidt > <simon.k.r.goldschmidt@gmail.com> wrote: >> >> This fixes the automatic lmb initialization and reservation for boards >> with more than one DRAM bank. >> >> This fixes the CVE-2018-18439 and -18440 fixes that only allowed to load >> files into the firs DRAM bank from fs and via tftp. >> >> Found-by: Heinrich Schuchardt <xypron.glpk@gmx.de> >> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> >> --- >> >> common/bootm.c | 4 ++-- >> fs/fs.c | 3 +-- >> include/lmb.h | 7 +++++-- >> lib/lmb.c | 37 ++++++++++++++++++++++++++++++++----- >> net/tftp.c | 3 +-- >> 5 files changed, 41 insertions(+), 13 deletions(-) > > Reviewed-by: Simon Glass <sjg@chromium.org> > > Do we need a test update to cover this? > > > - Simon > Currently in Travis we only check that loading to allowable regions works. This is where I experienced trouble that led to this patch. A test checking that loading to disallowed regions fails is not yet available. Unfortunately such a test would have to be board specific. Maybe we should just do a sandbox test. It would be helpful for such a test if the sandbox were using multiple DRAM banks. Currently the sandbox shows only a single DRAM bank of *zero* size: => bdinfo boot_params = 0x0000000000000000 DRAM bank = 0x0000000000000000 -> start = 0x0000000000000000 -> size = 0x0000000008000000 ethaddr = 00:00:11:22:33:44 IP addr = 1.2.3.4 @Simon Is this a display bug? Best regards Heinrich
On Thu, Jan 31, 2019 at 12:34 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 1/31/19 11:04 AM, Simon Glass wrote: > > Hi Simon, > > > > On Sat, 26 Jan 2019 at 14:13, Simon Goldschmidt > > <simon.k.r.goldschmidt@gmail.com> wrote: > >> > >> This fixes the automatic lmb initialization and reservation for boards > >> with more than one DRAM bank. > >> > >> This fixes the CVE-2018-18439 and -18440 fixes that only allowed to load > >> files into the firs DRAM bank from fs and via tftp. > >> > >> Found-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > >> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > >> --- > >> > >> common/bootm.c | 4 ++-- > >> fs/fs.c | 3 +-- > >> include/lmb.h | 7 +++++-- > >> lib/lmb.c | 37 ++++++++++++++++++++++++++++++++----- > >> net/tftp.c | 3 +-- > >> 5 files changed, 41 insertions(+), 13 deletions(-) > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > Do we need a test update to cover this? > > > > > > - Simon > > > Currently in Travis we only check that loading to allowable regions > works. This is where I experienced trouble that led to this patch. > > A test checking that loading to disallowed regions fails is not yet > available. Unfortunately such a test would have to be board specific. > Maybe we should just do a sandbox test. > > It would be helpful for such a test if the sandbox were using multiple > DRAM banks. > > Currently the sandbox shows only a single DRAM bank of *zero* size: > > => bdinfo > boot_params = 0x0000000000000000 > DRAM bank = 0x0000000000000000 > -> start = 0x0000000000000000 > -> size = 0x0000000008000000 But size is *not* zero here? > ethaddr = 00:00:11:22:33:44 > IP addr = 1.2.3.4 > > @Simon Which Simon? ;-) Regards, Simon > Is this a display bug? > > Best regards > > Heinrich
On Sat, Jan 26, 2019 at 10:13:04PM +0100, Simon Goldschmidt wrote: > This fixes the automatic lmb initialization and reservation for boards > with more than one DRAM bank. > > This fixes the CVE-2018-18439 and -18440 fixes that only allowed to load > files into the firs DRAM bank from fs and via tftp. > > Found-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > Tested-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > Reviewed-by: Simon Glass <sjg@chromium.org> Applied to u-boot/master, thanks!
diff --git a/common/bootm.c b/common/bootm.c index a4618b6d2e..7c7505f092 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -59,8 +59,8 @@ static void boot_start_lmb(bootm_headers_t *images) mem_start = env_get_bootm_low(); mem_size = env_get_bootm_size(); - lmb_init_and_reserve(&images->lmb, (phys_addr_t)mem_start, mem_size, - NULL); + lmb_init_and_reserve_range(&images->lmb, (phys_addr_t)mem_start, + mem_size, NULL); } #else #define lmb_reserve(lmb, base, size) diff --git a/fs/fs.c b/fs/fs.c index 7fd22101ef..9a411c04e2 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -453,8 +453,7 @@ static int fs_read_lmb_check(const char *filename, ulong addr, loff_t offset, 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_init_and_reserve(&lmb, gd->bd, (void *)gd->fdt_blob); lmb_dump_all(&lmb); if (lmb_alloc_addr(&lmb, addr, read_len) == addr) diff --git a/include/lmb.h b/include/lmb.h index 1bb003e35e..ca235d7d9d 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -4,6 +4,8 @@ #ifdef __KERNEL__ #include <asm/types.h> +#include <asm/u-boot.h> + /* * Logical memory blocks. * @@ -29,8 +31,9 @@ struct 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 void lmb_init_and_reserve(struct lmb *lmb, bd_t *bd, void *fdt_blob); +extern void lmb_init_and_reserve_range(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 3407705fa7..b210c2adaa 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -98,12 +98,8 @@ 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) +static void lmb_reserve_common(struct lmb *lmb, void *fdt_blob) { - lmb_init(lmb); - lmb_add(lmb, base, size); arch_lmb_reserve(lmb); board_lmb_reserve(lmb); @@ -111,6 +107,37 @@ void lmb_init_and_reserve(struct lmb *lmb, phys_addr_t base, phys_size_t size, boot_fdt_add_mem_rsv_regions(lmb, fdt_blob); } +/* Initialize the struct, add memory and call arch/board reserve functions */ +void lmb_init_and_reserve(struct lmb *lmb, bd_t *bd, void *fdt_blob) +{ +#ifdef CONFIG_NR_DRAM_BANKS + int i; +#endif + + lmb_init(lmb); +#ifdef CONFIG_NR_DRAM_BANKS + for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) { + if (bd->bi_dram[i].size) { + lmb_add(lmb, bd->bi_dram[i].start, + bd->bi_dram[i].size); + } + } +#else + if (bd->bi_memsize) + lmb_add(lmb, bd->bi_memstart, bd->bi_memsize); +#endif + lmb_reserve_common(lmb, fdt_blob); +} + +/* Initialize the struct, add memory and call arch/board reserve functions */ +void lmb_init_and_reserve_range(struct lmb *lmb, phys_addr_t base, + phys_size_t size, void *fdt_blob) +{ + lmb_init(lmb); + lmb_add(lmb, base, size); + lmb_reserve_common(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) { diff --git a/net/tftp.c b/net/tftp.c index 8fab6d2650..75f3a7c141 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -606,8 +606,7 @@ static int tftp_init_load_addr(void) struct lmb lmb; phys_size_t max_size; - lmb_init_and_reserve(&lmb, gd->bd->bi_dram[0].start, - gd->bd->bi_dram[0].size, (void *)gd->fdt_blob); + lmb_init_and_reserve(&lmb, gd->bd, (void *)gd->fdt_blob); max_size = lmb_get_unreserved_size(&lmb, load_addr); if (!max_size)
This fixes the automatic lmb initialization and reservation for boards with more than one DRAM bank. This fixes the CVE-2018-18439 and -18440 fixes that only allowed to load files into the firs DRAM bank from fs and via tftp. Found-by: Heinrich Schuchardt <xypron.glpk@gmx.de> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> --- common/bootm.c | 4 ++-- fs/fs.c | 3 +-- include/lmb.h | 7 +++++-- lib/lmb.c | 37 ++++++++++++++++++++++++++++++++----- net/tftp.c | 3 +-- 5 files changed, 41 insertions(+), 13 deletions(-)