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

[Simon Goldschmidt]

Am 26.01.2019 um 14:17 schrieb Tom Rini:
> On Sat, Jan 26, 2019 at 09:46:35AM +0100, Simon Goldschmidt wrote:
>> Am 26.01.2019 um 04:20 schrieb 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.
>>
>> OK, that's probably not expected ;-)
>>
>> I'd appreciate it if you could continue to track this down to get it fixed.
>>
>>>
>>> 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
>>
>> I don't know the first. The latter 2 are not initialized yet in this error
>> path and so are expected to be zero here.
>>
>> Could you run that test again if I sent you a patch enabling required output
>> for me to debug this?
>>
>>>
>>> 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.
>>
>> You're right, it should compile without CONFIG_LMB. It did initially, so I
>> guess that got lost somewhere during all the versions until v10, sorry. I'll
>> work on that.
> 
> That might be on me.  There were a few cases in the networking code
> where the patch broke building the existing world.

Trying again to compile with CONFIG_LMB disabled, it didn't work at all. 
It failed in places none of us touched for about 8 years, so I don't 
think it was you.

OTOH, I don't know what I had been testing to think it works with 
CONFIG_LMB disabled. I had to disable quite a few commands and features 
to keep it compiling.

In the end, I think we'll have to decide if we want to make it work with 
CONFIG_LMB disabled or if we make this mandatory.

What I did see is that some of the architectures don't overwrite 
'arch_lmb_reserve' and are thus probably still affected by these CVEs...

Regards,
Simon