[U-Boot] Strange CFI flash problem

[Kees Cook]

On Tue, Apr 15, 2014 at 10:27 AM, Kees Cook <keescook at chromium.org> wrote:
> On Mon, Apr 14, 2014 at 10:48 PM, Matthias Weißer <weisserm at arcor.de> wrote:
>> Am 14.04.2014 17:38, schrieb Kees Cook:
>>
>>> On Mon, Apr 14, 2014 at 1:51 AM, Matthias Weißer <weisserm at arcor.de>
>>> wrote:
>>>>
>>>> Am 14.04.2014 08:09, schrieb Matthias Weißer:
>>>>
>>>>> Hi Wolfgang
>>>>>
>>>>> Am 11.04.2014 12:43, schrieb Wolfgang Denk:
>>>>>>
>>>>>>
>>>>>> Dear Matthias,
>>>>>>
>>>>>> In message <5347BBBC.9000806 at arcor.de> you wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> we are currently trying to get an out-of-tree board based on 2013.01
>>>>>>> back in sync with current master and observing a strange behavior
>>>>>>> which
>>>>>>> we think is located in the CFI flash system. If we load an image via
>>>>>>> tftp, copy it to flash and then try to run the image via bootm we see
>>>>>>> an
>>>>>>> error while decomressing:
>>>>>>
>>>>>>
>>>>>> ...
>>>>>>>
>>>>>>>
>>>>>>>     Uncompressing Kernel Image ... LZO: uncompress or overwrite error
>>>>>>> -5
>>>>>>
>>>>>>
>>>>>>
>>>>>> Are you sure your malloc arena is big enough for LZO?  Try if
>>>>>> increasing it helps...
>>>>>
>>>>>
>>>>>
>>>>> We increaded it from 4MB to 8MB and the behavior is still the same.
>>>>>
>>>>> We also observed a different behavior when tftping the image to RAM and
>>>>> then directly executing it without copying it to flash. It seems that
>>>>> the flash device (EN29GL256H) is then in some a mode (maybe auto-select)
>>>>> which prevents it from normal read operations which doesn't allow the
>>>>> flash driver of the OS come up. We never saw this with our old u-boot.
>>>>> If there are no ideas left we will have to bisect the problem.
>>>>
>>>>
>>>>
>>>> Bisecting was successfull. The commit introducing the problem is
>>>>
>>>> commit ff9d2efdbf1b3b5263f81e843c6724b8bead7f1f
>>>> Author: Kees Cook <keescook at chromium.org>
>>>> Date:   Fri Aug 16 07:59:15 2013 -0700
>>>>
>>>>     lzo: correctly bounds-check output buffer
>>>>
>>>>     This checks the size of the output buffer and fails if it was going
>>>> to
>>>>     overflow the buffer during lzo decompression.
>>>>
>>>>     Signed-off-by: Kees Cook <keescook at chromium.org>
>>>>     Acked-by: Simon Glass <sjg at chromium.org>
>>>>
>>>> This commit introduced the usage of the dst_len output parameter as
>>>> additional input parameter containing the maximum output buffer size.
>>>> This
>>>> parameter isn't initialized in cmd_bootm.c:
>>>>
>>>>  454 #ifdef CONFIG_LZO
>>>>  455     case IH_COMP_LZO: {
>>>>  456         size_t size;
>>>>  457
>>>>  458         printf("   Uncompressing %s ... ", type_name);
>>>>  459
>>>>  460         ret = lzop_decompress(image_buf, image_len, load_buf,
>>>> &size);
>>>>
>>>> Setting size to some big value (SZE_MAX is not avialable) fixed the
>>>> behavior
>>>> but I am unsure if this is the correct solution. I think its hard to get
>>>> the
>>>> max output buffer size at this point in cmd_bootm.c.
>>>
>>>
>>> Does this work?
>>
>>
>> Yes. Didn't saw that configuration option. Thanks.
>>
>>
>>> ---
>>> From: Kees Cook <keescook at chromium.org>
>>> Subject: [PATCH] bootm: set max output for LZO
>>>
>>> The LZO decompressor wasn't initializing the maximum output size.
>>>
>>> Reported-by: Matthias Weißer <weisserm at arcor.de>
>>> Signed-off-by: Kees Cook <keescook at chromium.org>
>>> ---
>>>   common/cmd_bootm.c |    2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
>>> index 9751edc..c243a5b 100644
>>> --- a/common/cmd_bootm.c
>>> +++ b/common/cmd_bootm.c
>>> @@ -453,7 +453,7 @@ static int bootm_load_os(bootm_headers_t *images,
>>> unsigned long *load_end,
>>>   #endif /* CONFIG_LZMA */
>>>   #ifdef CONFIG_LZO
>>>          case IH_COMP_LZO: {
>>> -               size_t size;
>>> +               size_t size = unc_len;
>>>
>>>                  printf("   Uncompressing %s ... ", type_name);
>>>
>>
>> Tested-by: Matthias Weißer <weisserm at arcor.de>
>
> Great! Thanks for testing.
>
> Who can commit this patch? LZO is pretty busted at the moment without it. :)

Actually, since this is on -users, I'll resend the patch to the devel list...

Thanks!

-Kees

-- 
Kees Cook
Chrome OS Security