[PATCH v2] lz4: fix decompressor on big-endian powerpc

[Tom Rini]

On Sun, Jun 07, 2020 at 02:29:18PM +0200, Rasmus Villemoes wrote:

> Booting an lz4-compressed kernel image fails on our powerpc board with
> -EPROTONOSUPPORT. Adding a bit of debug prints, we get
> 
>   magic: 0x184d2204
>   flags: 0x64
>   reserved0: 1
>   has_content_checksum: 1
>   has_content_size: 0
>   has_block_checksum: 0
>   independent_blocks: 1
>   version: 0
>   block_descriptor: 70
>   reserved1: 7
>   max_block_size: 0
>   reserved2: 0
> 
> So the magic is ok, but the version check fails, also some reserved
> bits are apparently set. But that's because the code interprets the
> "flags" and "block_descriptor" bytes wrongly:
> 
> Using bit-fields to access individual bits of an "on the wire" format
> is not portable, not even when restricted to the C flavour implemented
> by gcc. Quoting the gcc manual:
> 
>    * 'The order of allocation of bit-fields within a unit (C90 6.5.2.1,
>      C99 and C11 6.7.2.1).'
> 
>      Determined by ABI.
> 
> and indeed, the PPC Processor ABI supplement says
> 
>    * Bit-fields are allocated from right to left (least to most
>      significant) on Little-Endian implementations and from left to
>      right (most to least significant) on Big-Endian implementations.
> 
> The upstream code (github.com/lz4/lz4) uses explicit shifts and masks
> for encoding/decoding:
> 
>     /* FLG Byte */
>     *dstPtr++ = (BYTE)(((1 & _2BITS) << 6)    /* Version('01') */
>         + ((cctxPtr->prefs.frameInfo.blockMode & _1BIT ) << 5)
>         + ((cctxPtr->prefs.frameInfo.blockChecksumFlag & _1BIT ) << 4)
>         + ((unsigned)(cctxPtr->prefs.frameInfo.contentSize > 0) << 3)
>         + ((cctxPtr->prefs.frameInfo.contentChecksumFlag & _1BIT ) << 2)
>         +  (cctxPtr->prefs.frameInfo.dictID > 0) );
> 
>     /* Flags */
>     {   U32 const FLG = srcPtr[4];
>         U32 const version = (FLG>>6) & _2BITS;
>         blockChecksumFlag = (FLG>>4) & _1BIT;
>         blockMode = (FLG>>5) & _1BIT;
>         contentSizeFlag = (FLG>>3) & _1BIT;
>         contentChecksumFlag = (FLG>>2) & _1BIT;
>         dictIDFlag = FLG & _1BIT;
>         /* validate */
>         if (((FLG>>1)&_1BIT) != 0) return err0r(LZ4F_ERROR_reservedFlag_set); /* Reserved bit */
>         if (version != 1) return err0r(LZ4F_ERROR_headerVersion_wrong);        /* Version Number, only supported value */
>     }
> 
> Do the same here, and while at it, be more careful to use unaligned
> accessors to what is most likely unaligned. Also update the comment to
> make it clear that it only refers to the lz4.c file, not the following
> code of lz4_wrapper.c.
> 
> This has been tested partly, of course, by seeing that my
> lz4-compressed kernel now boots, partly by running the (de)compression
> test-suite in the (x86_64) sandbox - i.e., it should still work just
> fine on little-endian hosts.
> 
> Reviewed-by: Julius Werner <jwerner at chromium.org>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200717/b5f021fd/attachment.sig>