From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.3 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_2 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7DB2CC43331 for ; Thu, 26 Mar 2020 09:03:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5532D2070A for ; Thu, 26 Mar 2020 09:03:24 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=nic.cz header.i=@nic.cz header.b="sRyDoPXZ" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727781AbgCZJDX (ORCPT ); Thu, 26 Mar 2020 05:03:23 -0400 Received: from mail.nic.cz ([217.31.204.67]:34918 "EHLO mail.nic.cz" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726359AbgCZJDX (ORCPT ); Thu, 26 Mar 2020 05:03:23 -0400 Received: from localhost (unknown [172.20.6.135]) by mail.nic.cz (Postfix) with ESMTPSA id A9CC4142F70; Thu, 26 Mar 2020 10:03:21 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=nic.cz; s=default; t=1585213401; bh=B+yrrJyOjQAzfO3LFkEcPEy0hFVSrWmIa3NVGHnCHq4=; h=Date:From:To; b=sRyDoPXZV1rwYwRWynvcF1U1918e1nRs/c+aCz2bs3R83WHwPZomuGNgRi/ftGtlp bS5Q3hpcqDLv8sUGHf5XAY0p1Am4KzbwolvadNKHYHAsPDRT6RDiRYA8gcCguXdaaW IHEvI1IixRn8wF+q+CzFrisqEaDSiktQ5AWqJg08= Date: Thu, 26 Mar 2020 10:03:21 +0100 From: Marek Behun To: Qu Wenruo Cc: u-boot@lists.denx.de, linux-btrfs@vger.kernel.org Subject: Re: [PATCH U-BOOT v2 3/3] fs: btrfs: Fix LZO false decompression error caused by pending zero Message-ID: <20200326100321.6a854808@nic.cz> In-Reply-To: <20200326053556.20492-4-wqu@suse.com> References: <20200326053556.20492-1-wqu@suse.com> <20200326053556.20492-4-wqu@suse.com> X-Mailer: Claws Mail 3.17.3 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Virus-Scanned: clamav-milter 0.101.4 at mail X-Virus-Status: Clean Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org On Thu, 26 Mar 2020 13:35:56 +0800 Qu Wenruo wrote: > For certain btrfs files with compressed file extent, uboot will fail to > load it: >=20 > btrfs_read_extent_reg: disk_bytenr=3D14229504 disk_len=3D73728 offset= =3D0 nr_bytes=3D131 > 072 > decompress_lzo: tot_len=3D70770 > decompress_lzo: in_len=3D1389 > decompress_lzo: in_len=3D2400 > decompress_lzo: in_len=3D3002 > decompress_lzo: in_len=3D1379 > decompress_lzo: in_len=3D88539136 > decompress_lzo: header error, in_len=3D88539136 clen=3D65534 tot_len=3D= 62580 >=20 > NOTE: except the last line, all other lines are debug output. >=20 > Btrfs lzo compression uses its own format to record compressed size > (segment header, LE32). >=20 > However to make decompression easier, we never put such segment header > across page boundary. >=20 > In above case, the xxd dump of the lzo compressed data looks like this: >=20 > 00001fe0: 4cdc 02fc 0bfd 02c0 dc02 0d13 0100 0001 L............... > 00001ff0: 0000 0008 0300 0000 0000 0011 0000|0000 ................ > 00002000: 4705 0000 0001 cc02 0000 0000 0000 1e01 G............... >=20 > '|' is the "expected" segment header start position. >=20 > But in that page, there are only 2 bytes left, can't contain the 4 bytes > segment header. >=20 > So btrfs compression will skip that 2 bytes, put the segment header in > next page directly. >=20 > Uboot doesn't have such check, and read the header with 2 bytes offset, > result 0x05470000 (88539136), other than the expected result > 0x00000547 (1351), resulting above error. >=20 > Follow the btrfs-progs restore implementation, by introducing tot_in to > record total processed bytes (including headers), and do proper page > boundary skip to fix it. >=20 > Please note that, current code base doesn't parse fs_info thus we can't > grab sector size easily, so it uses PAGE_SIZE, and relying on fs open > time check to exclude unsupported sector size. >=20 > Signed-off-by: Qu Wenruo > Cc: Marek Behun > --- > fs/btrfs/compression.c | 20 +++++++++++++++++++- > 1 file changed, 19 insertions(+), 1 deletion(-) >=20 > diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c > index 4ef44ce11485..b1884fc15ee0 100644 > --- a/fs/btrfs/compression.c > +++ b/fs/btrfs/compression.c > @@ -9,6 +9,7 @@ > #include > #include > #include > +#include > #include > #include > =20 > @@ -16,7 +17,7 @@ > #define LZO_LEN 4 > static u32 decompress_lzo(const u8 *cbuf, u32 clen, u8 *dbuf, u32 dlen) > { > - u32 tot_len, in_len, res; > + u32 tot_len, tot_in, in_len, res; > size_t out_len; > int ret; > =20 > @@ -24,9 +25,11 @@ static u32 decompress_lzo(const u8 *cbuf, u32 clen, u8= *dbuf, u32 dlen) > return -1; > =20 > tot_len =3D le32_to_cpu(get_unaligned((u32 *)cbuf)); > + tot_in =3D 0; > cbuf +=3D LZO_LEN; > clen -=3D LZO_LEN; > tot_len -=3D LZO_LEN; > + tot_in +=3D LZO_LEN; > =20 > if (tot_len =3D=3D 0 && dlen) > return -1; > @@ -36,6 +39,8 @@ static u32 decompress_lzo(const u8 *cbuf, u32 clen, u8 = *dbuf, u32 dlen) > res =3D 0; > =20 > while (tot_len > LZO_LEN) { > + u32 rem_page; > + > in_len =3D le32_to_cpu(get_unaligned((u32 *)cbuf)); > cbuf +=3D LZO_LEN; > clen -=3D LZO_LEN; > @@ -44,6 +49,7 @@ static u32 decompress_lzo(const u8 *cbuf, u32 clen, u8 = *dbuf, u32 dlen) > return -1; > =20 > tot_len -=3D (LZO_LEN + in_len); > + tot_in +=3D (LZO_LEN + in_len); > =20 > out_len =3D dlen; > ret =3D lzo1x_decompress_safe(cbuf, in_len, dbuf, &out_len); > @@ -56,6 +62,18 @@ static u32 decompress_lzo(const u8 *cbuf, u32 clen, u8= *dbuf, u32 dlen) > dlen -=3D out_len; > =20 > res +=3D out_len; > + > + /* > + * If the 4 bytes header does not fit to the rest of the page we > + * have to move to next one, or we read some garbage. > + */ > + rem_page =3D PAGE_SIZE - (tot_in % PAGE_SIZE); > + if (rem_page < LZO_LEN) { > + cbuf +=3D rem_page; > + tot_in +=3D rem_page; > + clen -=3D rem_page; > + tot_len -=3D rem_page; > + } > } > =20 > return res; Reviewed-by: Marek Beh=C3=BAn From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Behun Date: Thu, 26 Mar 2020 10:03:21 +0100 Subject: [PATCH U-BOOT v2 3/3] fs: btrfs: Fix LZO false decompression error caused by pending zero In-Reply-To: <20200326053556.20492-4-wqu@suse.com> References: <20200326053556.20492-1-wqu@suse.com> <20200326053556.20492-4-wqu@suse.com> Message-ID: <20200326100321.6a854808@nic.cz> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Thu, 26 Mar 2020 13:35:56 +0800 Qu Wenruo wrote: > For certain btrfs files with compressed file extent, uboot will fail to > load it: > > btrfs_read_extent_reg: disk_bytenr=14229504 disk_len=73728 offset=0 nr_bytes=131 > 072 > decompress_lzo: tot_len=70770 > decompress_lzo: in_len=1389 > decompress_lzo: in_len=2400 > decompress_lzo: in_len=3002 > decompress_lzo: in_len=1379 > decompress_lzo: in_len=88539136 > decompress_lzo: header error, in_len=88539136 clen=65534 tot_len=62580 > > NOTE: except the last line, all other lines are debug output. > > Btrfs lzo compression uses its own format to record compressed size > (segment header, LE32). > > However to make decompression easier, we never put such segment header > across page boundary. > > In above case, the xxd dump of the lzo compressed data looks like this: > > 00001fe0: 4cdc 02fc 0bfd 02c0 dc02 0d13 0100 0001 L............... > 00001ff0: 0000 0008 0300 0000 0000 0011 0000|0000 ................ > 00002000: 4705 0000 0001 cc02 0000 0000 0000 1e01 G............... > > '|' is the "expected" segment header start position. > > But in that page, there are only 2 bytes left, can't contain the 4 bytes > segment header. > > So btrfs compression will skip that 2 bytes, put the segment header in > next page directly. > > Uboot doesn't have such check, and read the header with 2 bytes offset, > result 0x05470000 (88539136), other than the expected result > 0x00000547 (1351), resulting above error. > > Follow the btrfs-progs restore implementation, by introducing tot_in to > record total processed bytes (including headers), and do proper page > boundary skip to fix it. > > Please note that, current code base doesn't parse fs_info thus we can't > grab sector size easily, so it uses PAGE_SIZE, and relying on fs open > time check to exclude unsupported sector size. > > Signed-off-by: Qu Wenruo > Cc: Marek Behun > --- > fs/btrfs/compression.c | 20 +++++++++++++++++++- > 1 file changed, 19 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c > index 4ef44ce11485..b1884fc15ee0 100644 > --- a/fs/btrfs/compression.c > +++ b/fs/btrfs/compression.c > @@ -9,6 +9,7 @@ > #include > #include > #include > +#include > #include > #include > > @@ -16,7 +17,7 @@ > #define LZO_LEN 4 > static u32 decompress_lzo(const u8 *cbuf, u32 clen, u8 *dbuf, u32 dlen) > { > - u32 tot_len, in_len, res; > + u32 tot_len, tot_in, in_len, res; > size_t out_len; > int ret; > > @@ -24,9 +25,11 @@ static u32 decompress_lzo(const u8 *cbuf, u32 clen, u8 *dbuf, u32 dlen) > return -1; > > tot_len = le32_to_cpu(get_unaligned((u32 *)cbuf)); > + tot_in = 0; > cbuf += LZO_LEN; > clen -= LZO_LEN; > tot_len -= LZO_LEN; > + tot_in += LZO_LEN; > > if (tot_len == 0 && dlen) > return -1; > @@ -36,6 +39,8 @@ static u32 decompress_lzo(const u8 *cbuf, u32 clen, u8 *dbuf, u32 dlen) > res = 0; > > while (tot_len > LZO_LEN) { > + u32 rem_page; > + > in_len = le32_to_cpu(get_unaligned((u32 *)cbuf)); > cbuf += LZO_LEN; > clen -= LZO_LEN; > @@ -44,6 +49,7 @@ static u32 decompress_lzo(const u8 *cbuf, u32 clen, u8 *dbuf, u32 dlen) > return -1; > > tot_len -= (LZO_LEN + in_len); > + tot_in += (LZO_LEN + in_len); > > out_len = dlen; > ret = lzo1x_decompress_safe(cbuf, in_len, dbuf, &out_len); > @@ -56,6 +62,18 @@ static u32 decompress_lzo(const u8 *cbuf, u32 clen, u8 *dbuf, u32 dlen) > dlen -= out_len; > > res += out_len; > + > + /* > + * If the 4 bytes header does not fit to the rest of the page we > + * have to move to next one, or we read some garbage. > + */ > + rem_page = PAGE_SIZE - (tot_in % PAGE_SIZE); > + if (rem_page < LZO_LEN) { > + cbuf += rem_page; > + tot_in += rem_page; > + clen -= rem_page; > + tot_len -= rem_page; > + } > } > > return res; Reviewed-by: Marek Beh?n