From patchwork Sun Jun 7 12:29:18 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rasmus Villemoes X-Patchwork-Id: 1304681 X-Patchwork-Delegate: trini@ti.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.denx.de (client-ip=85.214.62.61; helo=phobos.denx.de; envelope-from=u-boot-bounces@lists.denx.de; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=prevas.dk Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=prevas.dk header.i=@prevas.dk header.a=rsa-sha256 header.s=selector1 header.b=TxXWHsBn; dkim-atps=neutral Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49fwgT6xQqz9sQx for ; Sun, 7 Jun 2020 22:29:41 +1000 (AEST) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 08286801CA; Sun, 7 Jun 2020 14:29:37 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=prevas.dk Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=prevas.dk header.i=@prevas.dk header.b="TxXWHsBn"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id C4666801D8; Sun, 7 Jun 2020 14:29:34 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on phobos.denx.de X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FORGED_SPF_HELO,MSGID_FROM_MTA_HEADER, SPF_HELO_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.2 Received: from EUR05-DB8-obe.outbound.protection.outlook.com (mail-db8eur05on2072c.outbound.protection.outlook.com [IPv6:2a01:111:f400:7e1a::72c]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id D786980199 for ; Sun, 7 Jun 2020 14:29:30 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=prevas.dk Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=rasmus.villemoes@prevas.dk ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=I+82ClyBkTune1fKy6LXhnAURgLI0kOAFnZHsha1wuYcTOp07n4Q4C2xwhMUhjfoDKaqRUS7dCmNwrigv1F8Pr++GsGRxgzVQo3BlaT123dgnTI4kA7PifcPcW/uF2n+crRxiIR//OacZ7m203IbhVCXa5xUlEOd2/50xqmndtNwDCAPBchWi9PwvO+CFmtmcZQqwtRsjPI+f+j1bOuqTD/1nc/9KYQ87ATsp5mlJUw85HRYqhQf9exNzbiWs2g8UcZKPmTbp21+tk5myeTMtq+B2xTNgfnBGJS/BrB8+aQM4y8Lo86L5X4e7V9wuMT5zkBDLqrf/SiwcElRmEAknQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=GR74r3k5BewocqhXUnPpD25v9emcUX6BRleNx1yK1KI=; b=VB08YRpNE5z7bb2N/lHk4BUDNS7Up37nKsKqsqvKvNzjiIQvAAU/IQyMN+FBaHTJN/dnFPez4BkdIn7b0yfP0yw4HvMGjSPxX01foK+ID98/Upao1FO+qNUFWPNJrxQzMwMhBxGOvufDf0zgGa0dEQa+p/9lbSFjUoBU1GiY+hjJIvySdk2SFpGWsRV5pExVGB4N+B9u8lzZgnrwN4C+Yld7MYn5PheK2nBgOGWi9V+1MBQ3fzVOgDtxm7J2VJxIyx98sS/AsPcJq9eGjFYhHyfeFBFllZGkP47L86fiQiFqJtl/dxBVr8tq5OZc/WzEohJxKtB/H5xkMMMpB3eW9w== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=prevas.dk; dmarc=pass action=none header.from=prevas.dk; dkim=pass header.d=prevas.dk; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=prevas.dk; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=GR74r3k5BewocqhXUnPpD25v9emcUX6BRleNx1yK1KI=; b=TxXWHsBn8T7UnEmyqDwHjkwXIaz9F4JGYF9dIVl2vryjbb7q4CMakd2W4rwF9dowR/oTzGsnVYPvn0qwD9Wt0K2EwuzYLgFyPVMVvPrpdVu51omVu4bSAcXEfvAb/D/4ZuAXPNn5l+3VjCIt1qaW7rp8h5WR4jXhLXcGZmMK29c= Authentication-Results: lists.denx.de; dkim=none (message not signed) header.d=none;lists.denx.de; dmarc=none action=none header.from=prevas.dk; Received: from VI1PR10MB2765.EURPRD10.PROD.OUTLOOK.COM (2603:10a6:803:e1::21) by VI1PR10MB2190.EURPRD10.PROD.OUTLOOK.COM (2603:10a6:803:7a::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3066.18; Sun, 7 Jun 2020 12:29:28 +0000 Received: from VI1PR10MB2765.EURPRD10.PROD.OUTLOOK.COM ([fe80::f0ac:4e97:2536:faa]) by VI1PR10MB2765.EURPRD10.PROD.OUTLOOK.COM ([fe80::f0ac:4e97:2536:faa%7]) with mapi id 15.20.3066.023; Sun, 7 Jun 2020 12:29:28 +0000 From: Rasmus Villemoes To: u-boot@lists.denx.de Cc: Julius Werner , Simon Glass , Tom Rini , Rasmus Villemoes Subject: [PATCH v2] lz4: fix decompressor on big-endian powerpc Date: Sun, 7 Jun 2020 14:29:18 +0200 Message-Id: <20200607122918.20886-1-rasmus.villemoes@prevas.dk> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20200605102920.1545-1-rasmus.villemoes@prevas.dk> References: <20200605102920.1545-1-rasmus.villemoes@prevas.dk> X-ClientProxiedBy: AM6P194CA0066.EURP194.PROD.OUTLOOK.COM (2603:10a6:209:84::43) To VI1PR10MB2765.EURPRD10.PROD.OUTLOOK.COM (2603:10a6:803:e1::21) MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from prevas-ravi.prevas.se (5.186.116.45) by AM6P194CA0066.EURP194.PROD.OUTLOOK.COM (2603:10a6:209:84::43) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3066.18 via Frontend Transport; Sun, 7 Jun 2020 12:29:27 +0000 X-Mailer: git-send-email 2.23.0 X-Originating-IP: [5.186.116.45] X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: bbd0c91c-fad9-4e50-ab70-08d80ade6bd5 X-MS-TrafficTypeDiagnostic: VI1PR10MB2190: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:4303; X-Forefront-PRVS: 04270EF89C X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 7eJ5fn1DD601qqqrwYeMgywBqhId7yJT0IZTXvLeX7QZZ6AESoBhOQmZj6a7hF6778RyWH/doFWjKvwj7r0G/Qb48jfF7kZ2CBPkUx9zQXEA4BrcYbieN9kppS3YaWQXykIJa1ihsB56XC696L2iINCMeyocy1gYf9qWCiCVOevWn5BB+tq7y2959TVDZeNMf69G7odff7zzlwMmZf60iTI/n9QFy0RBMNYPj+1uvzOaOIgaXLwxhLT3NEFZTJOvSNmxb0/R61UPKIOFT+1Z324H+wdox/PHxrRKKjqXMjIzIbp9l7F3kvtw15VeCTLklz91WkTdykHn+ctxOk8fIA== X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:VI1PR10MB2765.EURPRD10.PROD.OUTLOOK.COM; PTR:; CAT:NONE; SFTY:; SFS:(376002)(346002)(136003)(396003)(39830400003)(366004)(4326008)(478600001)(6916009)(36756003)(83380400001)(54906003)(316002)(2906002)(52116002)(107886003)(6666004)(44832011)(66946007)(8936002)(8976002)(1076003)(6512007)(956004)(6506007)(26005)(6486002)(2616005)(86362001)(66476007)(66556008)(5660300002)(186003)(8676002)(16526019); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData: FmOPwjyEzPQB7ENbyAQ2E9DNxsibPjqXVql9svw+ZQaF555wdrVPI88pHR+H7wLe6w3yoJMXFatMrGXz+CeSN3WRawzay48Z9pTOpOuJjDX9BUTR2E0i9SKTE2q19At5ZrIIJ1ZsZvu2F9aOs4MQq068Qbt6QLtZIWK3Whs1PtnUatkoXLFjHu6MlYxngoQhTvhpVxEr2+BRvIyGUjYJG9J4/c2QnBMIvxGhxRuH2BOiNGIo2rhml1NcKGNJFEL+axgUFB9h3bNc5Q0Y978U4NBwzB6zjfgSsCRQdbHP6zWpe+LVPrhZ2utDHkm3w7fum1aj5/4IWGkQniwEHi9hzSD8tZsq/ecqh+lb5LcLX1yGhKCaXKo8ih0XRNRa/x4gc9m/b5dramWJsUPgFOFFx5ooRsup4vViS2VG1MRIqP9Xo27XkUwkNjUGzR76lxc70jvE5Y0Ig0P+UbVygcqyjL2l1d81HymmtW/01/v8eIo= X-OriginatorOrg: prevas.dk X-MS-Exchange-CrossTenant-Network-Message-Id: bbd0c91c-fad9-4e50-ab70-08d80ade6bd5 X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Jun 2020 12:29:28.2846 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: d350cf71-778d-4780-88f5-071a4cb1ed61 X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: S462y7py14ZNQMD0mpmIkbWs1dhlCWS7+YQNHyJj4dY0dH/WYDHcBpcbyvCSq3COfGgWGotolSxAF8vIeCcpxnkS6UXBfevI/Qu/hj87/nE= X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR10MB2190 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.30rc1 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.102.2 at phobos.denx.de X-Virus-Status: Clean 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 Signed-off-by: Rasmus Villemoes --- v2: Update comment more accurately, adjust commit log, add Julius' R-b. lib/lz4_wrapper.c | 95 +++++++++++++++++++---------------------------- 1 file changed, 39 insertions(+), 56 deletions(-) diff --git a/lib/lz4_wrapper.c b/lib/lz4_wrapper.c index 1e1e8d5085..e0f7d3688e 100644 --- a/lib/lz4_wrapper.c +++ b/lib/lz4_wrapper.c @@ -9,6 +9,7 @@ #include #include #include +#include static u16 LZ4_readLE16(const void *src) { return le16_to_cpu(*(u16 *)src); } static void LZ4_copy4(void *dst, const void *src) { *(u32 *)dst = *(u32 *)src; } @@ -22,45 +23,10 @@ typedef uint64_t U64; #define FORCE_INLINE static inline __attribute__((always_inline)) -/* Unaltered (except removing unrelated code) from github.com/Cyan4973/lz4. */ +/* lz4.c is unaltered (except removing unrelated code) from github.com/Cyan4973/lz4. */ #include "lz4.c" /* #include for inlining, do not link! */ -struct lz4_frame_header { - u32 magic; - union { - u8 flags; - struct { - u8 reserved0:2; - u8 has_content_checksum:1; - u8 has_content_size:1; - u8 has_block_checksum:1; - u8 independent_blocks:1; - u8 version:2; - }; - }; - union { - u8 block_descriptor; - struct { - u8 reserved1:4; - u8 max_block_size:3; - u8 reserved2:1; - }; - }; - /* + u64 content_size iff has_content_size is set */ - /* + u8 header_checksum */ -} __packed; - -struct lz4_block_header { - union { - u32 raw; - struct { - u32 size:31; - u32 not_compressed:1; - }; - }; - /* + size bytes of data */ - /* + u32 block_checksum iff has_block_checksum is set */ -} __packed; +#define LZ4F_BLOCKUNCOMPRESSED_FLAG 0x80000000U int ulz4fn(const void *src, size_t srcn, void *dst, size_t *dstn) { @@ -72,53 +38,70 @@ int ulz4fn(const void *src, size_t srcn, void *dst, size_t *dstn) *dstn = 0; { /* With in-place decompression the header may become invalid later. */ - const struct lz4_frame_header *h = in; + u32 magic; + u8 flags, version, independent_blocks, has_content_size; + u8 block_desc; - if (srcn < sizeof(*h) + sizeof(u64) + sizeof(u8)) + if (srcn < sizeof(u32) + 3*sizeof(u8)) return -EINVAL; /* input overrun */ + magic = get_unaligned_le32(in); + in += sizeof(u32); + flags = *(u8 *)in; + in += sizeof(u8); + block_desc = *(u8 *)in; + in += sizeof(u8); + + version = (flags >> 6) & 0x3; + independent_blocks = (flags >> 5) & 0x1; + has_block_checksum = (flags >> 4) & 0x1; + has_content_size = (flags >> 3) & 0x1; + /* We assume there's always only a single, standard frame. */ - if (le32_to_cpu(h->magic) != LZ4F_MAGIC || h->version != 1) + if (magic != LZ4F_MAGIC || version != 1) return -EPROTONOSUPPORT; /* unknown format */ - if (h->reserved0 || h->reserved1 || h->reserved2) - return -EINVAL; /* reserved must be zero */ - if (!h->independent_blocks) + if ((flags & 0x03) || (block_desc & 0x8f)) + return -EINVAL; /* reserved bits must be zero */ + if (!independent_blocks) return -EPROTONOSUPPORT; /* we can't support this yet */ - has_block_checksum = h->has_block_checksum; - in += sizeof(*h); - if (h->has_content_size) + if (has_content_size) { + if (srcn < sizeof(u32) + 3*sizeof(u8) + sizeof(u64)) + return -EINVAL; /* input overrun */ in += sizeof(u64); + } + /* Header checksum byte */ in += sizeof(u8); } while (1) { - struct lz4_block_header b; + u32 block_header, block_size; - b.raw = le32_to_cpu(*(u32 *)in); - in += sizeof(struct lz4_block_header); + block_header = get_unaligned_le32(in); + in += sizeof(u32); + block_size = block_header & ~LZ4F_BLOCKUNCOMPRESSED_FLAG; - if (in - src + b.size > srcn) { + if (in - src + block_size > srcn) { ret = -EINVAL; /* input overrun */ break; } - if (!b.size) { + if (!block_size) { ret = 0; /* decompression successful */ break; } - if (b.not_compressed) { - size_t size = min((ptrdiff_t)b.size, end - out); + if (block_header & LZ4F_BLOCKUNCOMPRESSED_FLAG) { + size_t size = min((ptrdiff_t)block_size, end - out); memcpy(out, in, size); out += size; - if (size < b.size) { + if (size < block_size) { ret = -ENOBUFS; /* output overrun */ break; } } else { /* constant folding essential, do not touch params! */ - ret = LZ4_decompress_generic(in, out, b.size, + ret = LZ4_decompress_generic(in, out, block_size, end - out, endOnInputSize, full, 0, noDict, out, NULL, 0); if (ret < 0) { @@ -128,7 +111,7 @@ int ulz4fn(const void *src, size_t srcn, void *dst, size_t *dstn) out += ret; } - in += b.size; + in += block_size; if (has_block_checksum) in += sizeof(u32); }