lz4: fix decompressor on big-endian powerpc
authorRasmus Villemoes <rasmus.villemoes@prevas.dk>
Sun, 7 Jun 2020 12:29:18 +0000 (14:29 +0200)
committerTom Rini <trini@konsulko.com>
Fri, 17 Jul 2020 12:45:29 +0000 (08:45 -0400)
commite7885a48bb362a212e52a4abf09251c08fd29a04
tree5961c7a0358beb340008f91dfda8ebaa3d32b7f4
parentaac9f284dbc3ae6a14d6fc8cf06c89aa9431b75b
lz4: fix decompressor on big-endian powerpc

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@chromium.org>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
lib/lz4_wrapper.c