[v2] zlib: Fix big performance regression

Michal Simek michal.simek at amd.com
Wed Jul 17 08:55:25 CEST 2024



On 7/16/24 16:35, Tom Rini wrote:
> From: Christophe Leroy <christophe.leroy at csgroup.eu>
> 
> Commit 340fdf1303dc ("zlib: Port fix for CVE-2016-9841 to U-Boot")
> brings a big performance regression in inflate_fast(), which leads
> to watchdog timer reset on powerpc 8xx.
> 
> It looks like that commit does more than what it describe, it
> especially removed an important optimisation that was doing copies
> using halfwords instead of bytes. That unexpected change multiplied
> by almost 4 the time spent in inflate_fast() and increased by 40%
> the overall time needed to uncompress linux kernel image.
> 
> So partially revert that commit but keep post incrementation as it
> is the initial purpose of said commit.
> 
> Fixes: 340fdf1303dc ("zlib: Port fix for CVE-2016-9841 to U-Boot")
> [trini: Combine assorted patches in to this one, just restoring the
>          performance commit]
> Signed-off-by: Tom Rini <trini at konsulko.com>
> Signed-off-by: Christophe Leroy <christophe.leroy at csgroup.eu>
> ---
> Given that in master we now have Michal's series un-reverted, I have
> collapsed Christophe's series down to a patch that is just fixing the
> performance issues, while not regressing other platforms.
> 
> Cc: Michal Simek <michal.simek at amd.com>
> ---
>   lib/zlib/inffast.c | 51 ++++++++++++++++++++++++++++++++++++----------
>   lib/zlib/zlib.h    |  1 -
>   2 files changed, 40 insertions(+), 12 deletions(-)
> 
> diff --git a/lib/zlib/inffast.c b/lib/zlib/inffast.c
> index 5e2a65ad4d27..b5a0adcce69f 100644
> --- a/lib/zlib/inffast.c
> +++ b/lib/zlib/inffast.c
> @@ -236,18 +236,47 @@ unsigned start;         /* inflate()'s starting value for strm->avail_out */
>                       }
>                   }
>                   else {
> +		    unsigned short *sout;
> +		    unsigned long loops;
> +
>                       from = out - dist;          /* copy direct from output */
> -                    do {                        /* minimum length is three */
> -                        *out++ = *from++;
> -                        *out++ = *from++;
> -                        *out++ = *from++;
> -                        len -= 3;
> -                    } while (len > 2);
> -                    if (len) {
> -                        *out++ = *from++;
> -                        if (len > 1)
> -                            *out++ = *from++;
> -                    }
> +                    /* minimum length is three */
> +		    /* Align out addr */
> +		    if (!((long)(out - 1) & 1)) {
> +			*out++ = *from++;
> +			len--;
> +		    }
> +		    sout = (unsigned short *)out;
> +		    if (dist > 2 ) {
> +			unsigned short *sfrom;
> +
> +			sfrom = (unsigned short *)from;
> +			loops = len >> 1;
> +			do
> +			    *sout++ = get_unaligned(sfrom++);
> +			while (--loops);
> +			out = (unsigned char *)sout;
> +			from = (unsigned char *)sfrom;
> +		    } else { /* dist == 1 or dist == 2 */
> +			unsigned short pat16;
> +
> +			pat16 = *(sout - 1);
> +			if (dist == 1)
> +#if defined(__BIG_ENDIAN)
> +			    pat16 = (pat16 & 0xff) | ((pat16 & 0xff ) << 8);
> +#elif defined(__LITTLE_ENDIAN)
> +			    pat16 = (pat16 & 0xff00) | ((pat16 & 0xff00 ) >> 8);
> +#else
> +#error __BIG_ENDIAN nor __LITTLE_ENDIAN is defined
> +#endif
> +			loops = len >> 1;
> +			do
> +			    *sout++ = pat16;
> +			while (--loops);
> +			out = (unsigned char *)sout;
> +		    }
> +		    if (len & 1)
> +			*out++ = *from++;
>                   }
>               }
>               else if ((op & 64) == 0) {          /* 2nd level distance code */
> diff --git a/lib/zlib/zlib.h b/lib/zlib/zlib.h
> index 560e7be97d3a..f9b2f69ac027 100644
> --- a/lib/zlib/zlib.h
> +++ b/lib/zlib/zlib.h
> @@ -10,7 +10,6 @@
>   /* avoid conflicts */
>   #undef OFF
>   #undef ASMINF
> -#undef POSTINC
>   #undef NO_GZIP
>   #define GUNZIP
>   #undef STDC

I am fine with whatever change which doesn't break functionality and address 
CVE. I took that code from upstream zlib version but I forget details why I 
removed this part of code.

Acked-by: Michal Simek <michal.simek at amd.com>

Thanks,
Michal


More information about the U-Boot mailing list