[U-Boot] [U-Boot, v10, 09/10] tftp: prevent overwriting reserved memory

[Heinrich Schuchardt]

TheOn 1/14/19 10:38 PM, Simon Goldschmidt wrote:
> This fixes CVE-2018-18439 ("insufficient boundary checks in network
> image boot") by using lmb to check for a valid range to store
> received blocks.
> 
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
> Acked-by: Joe Hershberger <joe.hershberger at ni.com>
> ---

Hello Simon,

due to this patch merged as a156c47e39ad7d00 on
vexpress_ca15_tc2_defconfig the command 'dhcp filename' always fails. It
was working in v2019.01

Same is true for other platforms, e.g. vexpress_ca9x4_defconfig.

I put in an extra printf() and got:
TFTP error: trying to overwrite reserved memory...
storeaddr 0, tftp_load_addr 0, tftp_load_size 0

It is not even possible to disable the checks by undefining CONFIG_LMB
because a compile error arises without CONFIG_LMB:

cmd/bootz.c:48:21: error: ‘bootm_headers_t’ {aka ‘struct bootm_headers’}
has no member named ‘lmb’

I think the code should compile if CONFIG_LMB is undefined.

Further for all boards 'dhcp filename' should be working after your
patch series if it was working before the patch series.

Why is CONFIG_LMB hard coded? Shouldn't we try to avoid any new hard
coded CONFIG symbols? Consider moving it to Kconfig.

The logic you use in tftp_init_load_addr() is problematic:

Essentially it allows loading via tftp only in a single region within
the first DRAM bank. Why shouldn't I load to the second DRAM bank?

Even in a single DRAM bank we will have several reserved regions and in
between them several allowable regions for loading.

The LMB tests do not even find all reserved regions. E.g. on x86_64 it
allows loading to 0x1000000 though this address is used as a reserved
region for PCI, loading to which leads to a crash.

@Tom
This LMB patch series stops us from straightening out the Python tests
for tftp to make efi-next build without Travis CI error. Please, advise
how to proceed.

Best regards

Heinrich