[U-Boot] [PATCH 3/6] gzip: correctly bounds-check output buffer

[Kees Cook]

On Fri, Nov 8, 2013 at 4:04 AM, Michal Simek <monstr at monstr.eu> wrote:
> Hi Kees,
>
> On 08/16/2013 04:59 PM, Kees Cook wrote:
>> The output buffer size must not be reset by the gzip decoder or there
>> is a risk of overflowing memory during decompression.
>>
>> Signed-off-by: Kees Cook <keescook at chromium.org>
>> Acked-by: Simon Glass <sjg at chromium.org>
>> ---
>>  lib/gunzip.c |    4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/gunzip.c b/lib/gunzip.c
>> index 9959781..35abfb3 100644
>> --- a/lib/gunzip.c
>> +++ b/lib/gunzip.c
>> @@ -89,13 +89,13 @@ int zunzip(void *dst, int dstlen, unsigned char *src, unsigned long *lenp,
>>       s.avail_out = dstlen;
>>       do {
>>               r = inflate(&s, Z_FINISH);
>> -             if (r != Z_STREAM_END && r != Z_BUF_ERROR && stoponerr == 1) {
>> +             if (stoponerr == 1 && r != Z_STREAM_END &&
>> +                 (s.avail_out == 0 || r != Z_BUF_ERROR)) {
>>                       printf("Error: inflate() returned %d\n", r);
>>                       inflateEnd(&s);
>>                       return -1;
>>               }
>>               s.avail_in = *lenp - offset - (int)(s.next_out - (unsigned char*)dst);
>> -             s.avail_out = dstlen;
>>       } while (r == Z_BUF_ERROR);
>>       *lenp = s.next_out - (unsigned char *) dst;
>>       inflateEnd(&s);
>>
>
> I have done u-boot upgrade to v2013.10 version and I see the problem with this patch
> when I am trying to boot my zynq image.
>
> After reverting this patch everything works as expected.

Eek, sorry this is causing you trouble!

> Here is the image I am using.
> http://www.monstr.eu/20131108-image.ub

Is there any way you can extract just the gzipped kernel from this
image? I'm not sure how to get at it from this .ub file.

> Below is the bootlog.
>
> Do you have any idea what can be wrong?
> [...]
> Uncompressing Kernel Image ... Error: inflate() returned -5
> GUNZIP: uncompress, out-of-mem or overwrite error - must RESET board to recover
> resetting ...

Either my change is failing to detect end-of-buffer correctly, or it
_is_, in which case this has uncovered an unsafe caller of gunzip.
This is after the "Uncompressing" message, so it's this caller:

        case IH_COMP_GZIP:
                printf("   Uncompressing %s ... ", type_name);
                if (gunzip(load_buf, unc_len, image_buf, &image_len) != 0) {
                        puts("GUNZIP: uncompress, out-of-mem or overwrite "
                                "error - must RESET board to recover\n");
                        if (boot_progress)
                                bootstage_error(BOOTSTAGE_ID_DECOMP_IMAGE);
                        return BOOTM_ERR_RESET;
                }

                *load_end = load + image_len;
                break;

If the uncompressed length of the kernel image is larger than
"unc_len", then this is catching a legitimate memory overflow. This is
entirely controlled by CONFIG_SYS_BOOTM_LEN. Is it possible this is
set too low for your build?

-Kees

-- 
Kees Cook
Chrome OS Security