All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH U-BOOT v2 0/3] fs: btrfs: Fix false LZO decompression error due to missing page boundary check
@ 2020-03-26  5:35 ` Qu Wenruo
  0 siblings, 0 replies; 20+ messages in thread
From: Qu Wenruo @ 2020-03-26  5:35 UTC (permalink / raw)
  To: u-boot; +Cc: linux-btrfs

There is a bug that uboot can't load LZO compressed data extent while
kernel can handle it without any problem.

It turns out to be a page boundary case. The 3nd patch is the proper
fix, cross-ported from btrfs-progs.

The first patch is just to make my eyes less hurt.
The second patch is to make sure the driver will reject sector size not
matching PAGE_SIZE.
This keeps the behavior the same as kernel, even in theory we could do
better in U-boot. This is just a temporary fix, before better btrfs
driver implemented.

I guess it's time to backport proper code from btrfs-progs, other than
using tons of immediate codes.

Changelog:
v2:
- Fix code style problems
- Add a new patch to reject non-page-sized sector size
  Since kernel does the same thing, and non-4K page size u-boot boards
  are really rare, it shouldn't be a big problem.

Qu Wenruo (3):
  fs: btrfs: Use LZO_LEN to replace immediate number
  fs: btrfs: Reject fs with sector size other than PAGE_SIZE
  fs: btrfs: Fix LZO false decompression error caused by pending zero

 fs/btrfs/compression.c | 42 +++++++++++++++++++++++++++++++-----------
 fs/btrfs/super.c       |  8 ++++++++
 2 files changed, 39 insertions(+), 11 deletions(-)

-- 
2.26.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH U-BOOT v2 0/3] fs: btrfs: Fix false LZO decompression error due to missing page boundary check
@ 2020-03-26  5:35 ` Qu Wenruo
  0 siblings, 0 replies; 20+ messages in thread
From: Qu Wenruo @ 2020-03-26  5:35 UTC (permalink / raw)
  To: u-boot

There is a bug that uboot can't load LZO compressed data extent while
kernel can handle it without any problem.

It turns out to be a page boundary case. The 3nd patch is the proper
fix, cross-ported from btrfs-progs.

The first patch is just to make my eyes less hurt.
The second patch is to make sure the driver will reject sector size not
matching PAGE_SIZE.
This keeps the behavior the same as kernel, even in theory we could do
better in U-boot. This is just a temporary fix, before better btrfs
driver implemented.

I guess it's time to backport proper code from btrfs-progs, other than
using tons of immediate codes.

Changelog:
v2:
- Fix code style problems
- Add a new patch to reject non-page-sized sector size
  Since kernel does the same thing, and non-4K page size u-boot boards
  are really rare, it shouldn't be a big problem.

Qu Wenruo (3):
  fs: btrfs: Use LZO_LEN to replace immediate number
  fs: btrfs: Reject fs with sector size other than PAGE_SIZE
  fs: btrfs: Fix LZO false decompression error caused by pending zero

 fs/btrfs/compression.c | 42 +++++++++++++++++++++++++++++++-----------
 fs/btrfs/super.c       |  8 ++++++++
 2 files changed, 39 insertions(+), 11 deletions(-)

-- 
2.26.0

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH U-BOOT v2 1/3] fs: btrfs: Use LZO_LEN to replace immediate number
  2020-03-26  5:35 ` Qu Wenruo
@ 2020-03-26  5:35   ` Qu Wenruo
  -1 siblings, 0 replies; 20+ messages in thread
From: Qu Wenruo @ 2020-03-26  5:35 UTC (permalink / raw)
  To: u-boot; +Cc: linux-btrfs, Marek Behun

Just a cleanup. These immediate numbers make my eyes hurt.

Signed-off-by: Qu Wenruo <wqu@suse.com>
Cc: Marek Behun <marek.behun@nic.cz>
---
 fs/btrfs/compression.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 346875d45a1b..4ef44ce11485 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -12,36 +12,38 @@
 #include <u-boot/zlib.h>
 #include <asm/unaligned.h>
 
+/* Header for each segment, LE32, recording the compressed size */
+#define LZO_LEN		4
 static u32 decompress_lzo(const u8 *cbuf, u32 clen, u8 *dbuf, u32 dlen)
 {
 	u32 tot_len, in_len, res;
 	size_t out_len;
 	int ret;
 
-	if (clen < 4)
+	if (clen < LZO_LEN)
 		return -1;
 
 	tot_len = le32_to_cpu(get_unaligned((u32 *)cbuf));
-	cbuf += 4;
-	clen -= 4;
-	tot_len -= 4;
+	cbuf += LZO_LEN;
+	clen -= LZO_LEN;
+	tot_len -= LZO_LEN;
 
 	if (tot_len == 0 && dlen)
 		return -1;
-	if (tot_len < 4)
+	if (tot_len < LZO_LEN)
 		return -1;
 
 	res = 0;
 
-	while (tot_len > 4) {
+	while (tot_len > LZO_LEN) {
 		in_len = le32_to_cpu(get_unaligned((u32 *)cbuf));
-		cbuf += 4;
-		clen -= 4;
+		cbuf += LZO_LEN;
+		clen -= LZO_LEN;
 
-		if (in_len > clen || tot_len < 4 + in_len)
+		if (in_len > clen || tot_len < LZO_LEN + in_len)
 			return -1;
 
-		tot_len -= 4 + in_len;
+		tot_len -= (LZO_LEN + in_len);
 
 		out_len = dlen;
 		ret = lzo1x_decompress_safe(cbuf, in_len, dbuf, &out_len);
-- 
2.26.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH U-BOOT v2 1/3] fs: btrfs: Use LZO_LEN to replace immediate number
@ 2020-03-26  5:35   ` Qu Wenruo
  0 siblings, 0 replies; 20+ messages in thread
From: Qu Wenruo @ 2020-03-26  5:35 UTC (permalink / raw)
  To: u-boot

Just a cleanup. These immediate numbers make my eyes hurt.

Signed-off-by: Qu Wenruo <wqu@suse.com>
Cc: Marek Behun <marek.behun@nic.cz>
---
 fs/btrfs/compression.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 346875d45a1b..4ef44ce11485 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -12,36 +12,38 @@
 #include <u-boot/zlib.h>
 #include <asm/unaligned.h>
 
+/* Header for each segment, LE32, recording the compressed size */
+#define LZO_LEN		4
 static u32 decompress_lzo(const u8 *cbuf, u32 clen, u8 *dbuf, u32 dlen)
 {
 	u32 tot_len, in_len, res;
 	size_t out_len;
 	int ret;
 
-	if (clen < 4)
+	if (clen < LZO_LEN)
 		return -1;
 
 	tot_len = le32_to_cpu(get_unaligned((u32 *)cbuf));
-	cbuf += 4;
-	clen -= 4;
-	tot_len -= 4;
+	cbuf += LZO_LEN;
+	clen -= LZO_LEN;
+	tot_len -= LZO_LEN;
 
 	if (tot_len == 0 && dlen)
 		return -1;
-	if (tot_len < 4)
+	if (tot_len < LZO_LEN)
 		return -1;
 
 	res = 0;
 
-	while (tot_len > 4) {
+	while (tot_len > LZO_LEN) {
 		in_len = le32_to_cpu(get_unaligned((u32 *)cbuf));
-		cbuf += 4;
-		clen -= 4;
+		cbuf += LZO_LEN;
+		clen -= LZO_LEN;
 
-		if (in_len > clen || tot_len < 4 + in_len)
+		if (in_len > clen || tot_len < LZO_LEN + in_len)
 			return -1;
 
-		tot_len -= 4 + in_len;
+		tot_len -= (LZO_LEN + in_len);
 
 		out_len = dlen;
 		ret = lzo1x_decompress_safe(cbuf, in_len, dbuf, &out_len);
-- 
2.26.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH U-BOOT v2 2/3] fs: btrfs: Reject fs with sector size other than PAGE_SIZE
  2020-03-26  5:35 ` Qu Wenruo
@ 2020-03-26  5:35   ` Qu Wenruo
  -1 siblings, 0 replies; 20+ messages in thread
From: Qu Wenruo @ 2020-03-26  5:35 UTC (permalink / raw)
  To: u-boot; +Cc: linux-btrfs, Marek Behun

Although in theory u-boot fs driver could easily support more sector
sizes, current code base doesn't have good enough way to grab sector
size yet.

This would cause problem for later LZO fixes which rely on sector size.

And considering that most u-boot boards are using 4K page size, which is
also the most common sector size for btrfs, rejecting fs with
non-page-sized sector size shouldn't cause much problem.

This should only be a quick fix before we implement better sector size
support.

Signed-off-by: Qu Wenruo <wqu@suse.com>
Cc: Marek Behun <marek.behun@nic.cz>
---
 fs/btrfs/super.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 2dc4a6fcd7a3..b693a073fc0b 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -7,6 +7,7 @@
 
 #include "btrfs.h"
 #include <memalign.h>
+#include <linux/compat.h>
 
 #define BTRFS_SUPER_FLAG_SUPP	(BTRFS_HEADER_FLAG_WRITTEN	\
 				 | BTRFS_HEADER_FLAG_RELOC	\
@@ -232,6 +233,13 @@ int btrfs_read_superblock(void)
 		return -1;
 	}
 
+	if (sb->sectorsize != PAGE_SIZE) {
+		printf(
+	"%s: Unsupported sector size (%u), only supports %u as sector size\n",
+			__func__, sb->sectorsize, PAGE_SIZE);
+		return -1;
+	}
+
 	if (btrfs_info.sb.num_devices != 1) {
 		printf("%s: Unsupported number of devices (%lli). This driver "
 		       "only supports filesystem on one device.\n", __func__,
-- 
2.26.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH U-BOOT v2 2/3] fs: btrfs: Reject fs with sector size other than PAGE_SIZE
@ 2020-03-26  5:35   ` Qu Wenruo
  0 siblings, 0 replies; 20+ messages in thread
From: Qu Wenruo @ 2020-03-26  5:35 UTC (permalink / raw)
  To: u-boot

Although in theory u-boot fs driver could easily support more sector
sizes, current code base doesn't have good enough way to grab sector
size yet.

This would cause problem for later LZO fixes which rely on sector size.

And considering that most u-boot boards are using 4K page size, which is
also the most common sector size for btrfs, rejecting fs with
non-page-sized sector size shouldn't cause much problem.

This should only be a quick fix before we implement better sector size
support.

Signed-off-by: Qu Wenruo <wqu@suse.com>
Cc: Marek Behun <marek.behun@nic.cz>
---
 fs/btrfs/super.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 2dc4a6fcd7a3..b693a073fc0b 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -7,6 +7,7 @@
 
 #include "btrfs.h"
 #include <memalign.h>
+#include <linux/compat.h>
 
 #define BTRFS_SUPER_FLAG_SUPP	(BTRFS_HEADER_FLAG_WRITTEN	\
 				 | BTRFS_HEADER_FLAG_RELOC	\
@@ -232,6 +233,13 @@ int btrfs_read_superblock(void)
 		return -1;
 	}
 
+	if (sb->sectorsize != PAGE_SIZE) {
+		printf(
+	"%s: Unsupported sector size (%u), only supports %u as sector size\n",
+			__func__, sb->sectorsize, PAGE_SIZE);
+		return -1;
+	}
+
 	if (btrfs_info.sb.num_devices != 1) {
 		printf("%s: Unsupported number of devices (%lli). This driver "
 		       "only supports filesystem on one device.\n", __func__,
-- 
2.26.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH U-BOOT v2 3/3] fs: btrfs: Fix LZO false decompression error caused by pending zero
  2020-03-26  5:35 ` Qu Wenruo
@ 2020-03-26  5:35   ` Qu Wenruo
  -1 siblings, 0 replies; 20+ messages in thread
From: Qu Wenruo @ 2020-03-26  5:35 UTC (permalink / raw)
  To: u-boot; +Cc: linux-btrfs, Marek Behun

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 <wqu@suse.com>
Cc: Marek Behun <marek.behun@nic.cz>
---
 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 <malloc.h>
 #include <linux/lzo.h>
 #include <linux/zstd.h>
+#include <linux/compat.h>
 #include <u-boot/zlib.h>
 #include <asm/unaligned.h>
 
@@ -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;
-- 
2.26.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH U-BOOT v2 3/3] fs: btrfs: Fix LZO false decompression error caused by pending zero
@ 2020-03-26  5:35   ` Qu Wenruo
  0 siblings, 0 replies; 20+ messages in thread
From: Qu Wenruo @ 2020-03-26  5:35 UTC (permalink / raw)
  To: u-boot

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 <wqu@suse.com>
Cc: Marek Behun <marek.behun@nic.cz>
---
 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 <malloc.h>
 #include <linux/lzo.h>
 #include <linux/zstd.h>
+#include <linux/compat.h>
 #include <u-boot/zlib.h>
 #include <asm/unaligned.h>
 
@@ -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;
-- 
2.26.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH U-BOOT v2 1/3] fs: btrfs: Use LZO_LEN to replace immediate number
  2020-03-26  5:35   ` Qu Wenruo
@ 2020-03-26  9:02     ` Marek Behun
  -1 siblings, 0 replies; 20+ messages in thread
From: Marek Behun @ 2020-03-26  9:02 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: u-boot, linux-btrfs

On Thu, 26 Mar 2020 13:35:54 +0800
Qu Wenruo <wqu@suse.com> wrote:

> Just a cleanup. These immediate numbers make my eyes hurt.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> Cc: Marek Behun <marek.behun@nic.cz>
> ---
>  fs/btrfs/compression.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index 346875d45a1b..4ef44ce11485 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -12,36 +12,38 @@
>  #include <u-boot/zlib.h>
>  #include <asm/unaligned.h>
>  
> +/* Header for each segment, LE32, recording the compressed size */
> +#define LZO_LEN		4
>  static u32 decompress_lzo(const u8 *cbuf, u32 clen, u8 *dbuf, u32 dlen)
>  {
>  	u32 tot_len, in_len, res;
>  	size_t out_len;
>  	int ret;
>  
> -	if (clen < 4)
> +	if (clen < LZO_LEN)
>  		return -1;
>  
>  	tot_len = le32_to_cpu(get_unaligned((u32 *)cbuf));
> -	cbuf += 4;
> -	clen -= 4;
> -	tot_len -= 4;
> +	cbuf += LZO_LEN;
> +	clen -= LZO_LEN;
> +	tot_len -= LZO_LEN;
>  
>  	if (tot_len == 0 && dlen)
>  		return -1;
> -	if (tot_len < 4)
> +	if (tot_len < LZO_LEN)
>  		return -1;
>  
>  	res = 0;
>  
> -	while (tot_len > 4) {
> +	while (tot_len > LZO_LEN) {
>  		in_len = le32_to_cpu(get_unaligned((u32 *)cbuf));
> -		cbuf += 4;
> -		clen -= 4;
> +		cbuf += LZO_LEN;
> +		clen -= LZO_LEN;
>  
> -		if (in_len > clen || tot_len < 4 + in_len)
> +		if (in_len > clen || tot_len < LZO_LEN + in_len)
>  			return -1;
>  
> -		tot_len -= 4 + in_len;
> +		tot_len -= (LZO_LEN + in_len);
>  
>  		out_len = dlen;
>  		ret = lzo1x_decompress_safe(cbuf, in_len, dbuf, &out_len);

Reviewed-by: Marek Behún <marek.behun@nic.cz>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH U-BOOT v2 1/3] fs: btrfs: Use LZO_LEN to replace immediate number
@ 2020-03-26  9:02     ` Marek Behun
  0 siblings, 0 replies; 20+ messages in thread
From: Marek Behun @ 2020-03-26  9:02 UTC (permalink / raw)
  To: u-boot

On Thu, 26 Mar 2020 13:35:54 +0800
Qu Wenruo <wqu@suse.com> wrote:

> Just a cleanup. These immediate numbers make my eyes hurt.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> Cc: Marek Behun <marek.behun@nic.cz>
> ---
>  fs/btrfs/compression.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index 346875d45a1b..4ef44ce11485 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -12,36 +12,38 @@
>  #include <u-boot/zlib.h>
>  #include <asm/unaligned.h>
>  
> +/* Header for each segment, LE32, recording the compressed size */
> +#define LZO_LEN		4
>  static u32 decompress_lzo(const u8 *cbuf, u32 clen, u8 *dbuf, u32 dlen)
>  {
>  	u32 tot_len, in_len, res;
>  	size_t out_len;
>  	int ret;
>  
> -	if (clen < 4)
> +	if (clen < LZO_LEN)
>  		return -1;
>  
>  	tot_len = le32_to_cpu(get_unaligned((u32 *)cbuf));
> -	cbuf += 4;
> -	clen -= 4;
> -	tot_len -= 4;
> +	cbuf += LZO_LEN;
> +	clen -= LZO_LEN;
> +	tot_len -= LZO_LEN;
>  
>  	if (tot_len == 0 && dlen)
>  		return -1;
> -	if (tot_len < 4)
> +	if (tot_len < LZO_LEN)
>  		return -1;
>  
>  	res = 0;
>  
> -	while (tot_len > 4) {
> +	while (tot_len > LZO_LEN) {
>  		in_len = le32_to_cpu(get_unaligned((u32 *)cbuf));
> -		cbuf += 4;
> -		clen -= 4;
> +		cbuf += LZO_LEN;
> +		clen -= LZO_LEN;
>  
> -		if (in_len > clen || tot_len < 4 + in_len)
> +		if (in_len > clen || tot_len < LZO_LEN + in_len)
>  			return -1;
>  
> -		tot_len -= 4 + in_len;
> +		tot_len -= (LZO_LEN + in_len);
>  
>  		out_len = dlen;
>  		ret = lzo1x_decompress_safe(cbuf, in_len, dbuf, &out_len);

Reviewed-by: Marek Beh?n <marek.behun@nic.cz>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH U-BOOT v2 2/3] fs: btrfs: Reject fs with sector size other than PAGE_SIZE
  2020-03-26  5:35   ` Qu Wenruo
@ 2020-03-26  9:03     ` Marek Behun
  -1 siblings, 0 replies; 20+ messages in thread
From: Marek Behun @ 2020-03-26  9:03 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: u-boot, linux-btrfs

On Thu, 26 Mar 2020 13:35:55 +0800
Qu Wenruo <wqu@suse.com> wrote:

> Although in theory u-boot fs driver could easily support more sector
> sizes, current code base doesn't have good enough way to grab sector
> size yet.
> 
> This would cause problem for later LZO fixes which rely on sector size.
> 
> And considering that most u-boot boards are using 4K page size, which is
> also the most common sector size for btrfs, rejecting fs with
> non-page-sized sector size shouldn't cause much problem.
> 
> This should only be a quick fix before we implement better sector size
> support.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> Cc: Marek Behun <marek.behun@nic.cz>
> ---
>  fs/btrfs/super.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 2dc4a6fcd7a3..b693a073fc0b 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -7,6 +7,7 @@
>  
>  #include "btrfs.h"
>  #include <memalign.h>
> +#include <linux/compat.h>
>  
>  #define BTRFS_SUPER_FLAG_SUPP	(BTRFS_HEADER_FLAG_WRITTEN	\
>  				 | BTRFS_HEADER_FLAG_RELOC	\
> @@ -232,6 +233,13 @@ int btrfs_read_superblock(void)
>  		return -1;
>  	}
>  
> +	if (sb->sectorsize != PAGE_SIZE) {
> +		printf(
> +	"%s: Unsupported sector size (%u), only supports %u as sector size\n",
> +			__func__, sb->sectorsize, PAGE_SIZE);
> +		return -1;
> +	}
> +
>  	if (btrfs_info.sb.num_devices != 1) {
>  		printf("%s: Unsupported number of devices (%lli). This driver "
>  		       "only supports filesystem on one device.\n", __func__,

Reviewed-by: Marek Behún <marek.behun@nic.cz>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH U-BOOT v2 2/3] fs: btrfs: Reject fs with sector size other than PAGE_SIZE
@ 2020-03-26  9:03     ` Marek Behun
  0 siblings, 0 replies; 20+ messages in thread
From: Marek Behun @ 2020-03-26  9:03 UTC (permalink / raw)
  To: u-boot

On Thu, 26 Mar 2020 13:35:55 +0800
Qu Wenruo <wqu@suse.com> wrote:

> Although in theory u-boot fs driver could easily support more sector
> sizes, current code base doesn't have good enough way to grab sector
> size yet.
> 
> This would cause problem for later LZO fixes which rely on sector size.
> 
> And considering that most u-boot boards are using 4K page size, which is
> also the most common sector size for btrfs, rejecting fs with
> non-page-sized sector size shouldn't cause much problem.
> 
> This should only be a quick fix before we implement better sector size
> support.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> Cc: Marek Behun <marek.behun@nic.cz>
> ---
>  fs/btrfs/super.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 2dc4a6fcd7a3..b693a073fc0b 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -7,6 +7,7 @@
>  
>  #include "btrfs.h"
>  #include <memalign.h>
> +#include <linux/compat.h>
>  
>  #define BTRFS_SUPER_FLAG_SUPP	(BTRFS_HEADER_FLAG_WRITTEN	\
>  				 | BTRFS_HEADER_FLAG_RELOC	\
> @@ -232,6 +233,13 @@ int btrfs_read_superblock(void)
>  		return -1;
>  	}
>  
> +	if (sb->sectorsize != PAGE_SIZE) {
> +		printf(
> +	"%s: Unsupported sector size (%u), only supports %u as sector size\n",
> +			__func__, sb->sectorsize, PAGE_SIZE);
> +		return -1;
> +	}
> +
>  	if (btrfs_info.sb.num_devices != 1) {
>  		printf("%s: Unsupported number of devices (%lli). This driver "
>  		       "only supports filesystem on one device.\n", __func__,

Reviewed-by: Marek Beh?n <marek.behun@nic.cz>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH U-BOOT v2 3/3] fs: btrfs: Fix LZO false decompression error caused by pending zero
  2020-03-26  5:35   ` Qu Wenruo
@ 2020-03-26  9:03     ` Marek Behun
  -1 siblings, 0 replies; 20+ messages in thread
From: Marek Behun @ 2020-03-26  9:03 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: u-boot, linux-btrfs

On Thu, 26 Mar 2020 13:35:56 +0800
Qu Wenruo <wqu@suse.com> 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 <wqu@suse.com>
> Cc: Marek Behun <marek.behun@nic.cz>
> ---
>  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 <malloc.h>
>  #include <linux/lzo.h>
>  #include <linux/zstd.h>
> +#include <linux/compat.h>
>  #include <u-boot/zlib.h>
>  #include <asm/unaligned.h>
>  
> @@ -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 <marek.behun@nic.cz>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH U-BOOT v2 3/3] fs: btrfs: Fix LZO false decompression error caused by pending zero
@ 2020-03-26  9:03     ` Marek Behun
  0 siblings, 0 replies; 20+ messages in thread
From: Marek Behun @ 2020-03-26  9:03 UTC (permalink / raw)
  To: u-boot

On Thu, 26 Mar 2020 13:35:56 +0800
Qu Wenruo <wqu@suse.com> 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 <wqu@suse.com>
> Cc: Marek Behun <marek.behun@nic.cz>
> ---
>  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 <malloc.h>
>  #include <linux/lzo.h>
>  #include <linux/zstd.h>
> +#include <linux/compat.h>
>  #include <u-boot/zlib.h>
>  #include <asm/unaligned.h>
>  
> @@ -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 <marek.behun@nic.cz>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH U-BOOT v2 1/3] fs: btrfs: Use LZO_LEN to replace immediate number
  2020-03-26  5:35   ` Qu Wenruo
@ 2020-04-17 21:08     ` Tom Rini
  -1 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2020-04-17 21:08 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: u-boot, linux-btrfs, Marek Behun

[-- Attachment #1: Type: text/plain, Size: 309 bytes --]

On Thu, Mar 26, 2020 at 01:35:54PM +0800, Qu Wenruo wrote:

> Just a cleanup. These immediate numbers make my eyes hurt.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> Cc: Marek Behun <marek.behun@nic.cz>
> Reviewed-by: Marek Behún <marek.behun@nic.cz>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH U-BOOT v2 1/3] fs: btrfs: Use LZO_LEN to replace immediate number
@ 2020-04-17 21:08     ` Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2020-04-17 21:08 UTC (permalink / raw)
  To: u-boot

On Thu, Mar 26, 2020 at 01:35:54PM +0800, Qu Wenruo wrote:

> Just a cleanup. These immediate numbers make my eyes hurt.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> Cc: Marek Behun <marek.behun@nic.cz>
> Reviewed-by: Marek Beh?n <marek.behun@nic.cz>

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/20200417/850a5bd3/attachment.sig>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH U-BOOT v2 2/3] fs: btrfs: Reject fs with sector size other than PAGE_SIZE
  2020-03-26  5:35   ` Qu Wenruo
@ 2020-04-17 21:09     ` Tom Rini
  -1 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2020-04-17 21:09 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: u-boot, linux-btrfs, Marek Behun

[-- Attachment #1: Type: text/plain, Size: 778 bytes --]

On Thu, Mar 26, 2020 at 01:35:55PM +0800, Qu Wenruo wrote:

> Although in theory u-boot fs driver could easily support more sector
> sizes, current code base doesn't have good enough way to grab sector
> size yet.
> 
> This would cause problem for later LZO fixes which rely on sector size.
> 
> And considering that most u-boot boards are using 4K page size, which is
> also the most common sector size for btrfs, rejecting fs with
> non-page-sized sector size shouldn't cause much problem.
> 
> This should only be a quick fix before we implement better sector size
> support.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> Cc: Marek Behun <marek.behun@nic.cz>
> Reviewed-by: Marek Behún <marek.behun@nic.cz>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH U-BOOT v2 2/3] fs: btrfs: Reject fs with sector size other than PAGE_SIZE
@ 2020-04-17 21:09     ` Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2020-04-17 21:09 UTC (permalink / raw)
  To: u-boot

On Thu, Mar 26, 2020 at 01:35:55PM +0800, Qu Wenruo wrote:

> Although in theory u-boot fs driver could easily support more sector
> sizes, current code base doesn't have good enough way to grab sector
> size yet.
> 
> This would cause problem for later LZO fixes which rely on sector size.
> 
> And considering that most u-boot boards are using 4K page size, which is
> also the most common sector size for btrfs, rejecting fs with
> non-page-sized sector size shouldn't cause much problem.
> 
> This should only be a quick fix before we implement better sector size
> support.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> Cc: Marek Behun <marek.behun@nic.cz>
> Reviewed-by: Marek Beh?n <marek.behun@nic.cz>

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/20200417/9c9d8337/attachment.sig>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH U-BOOT v2 3/3] fs: btrfs: Fix LZO false decompression error caused by pending zero
  2020-03-26  5:35   ` Qu Wenruo
@ 2020-04-17 21:09     ` Tom Rini
  -1 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2020-04-17 21:09 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: u-boot, linux-btrfs, Marek Behun

[-- Attachment #1: Type: text/plain, Size: 2112 bytes --]

On Thu, Mar 26, 2020 at 01:35:56PM +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 <wqu@suse.com>
> Cc: Marek Behun <marek.behun@nic.cz>
> Reviewed-by: Marek Behún <marek.behun@nic.cz>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH U-BOOT v2 3/3] fs: btrfs: Fix LZO false decompression error caused by pending zero
@ 2020-04-17 21:09     ` Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2020-04-17 21:09 UTC (permalink / raw)
  To: u-boot

On Thu, Mar 26, 2020 at 01:35:56PM +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 <wqu@suse.com>
> Cc: Marek Behun <marek.behun@nic.cz>
> Reviewed-by: Marek Beh?n <marek.behun@nic.cz>

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/20200417/cf66fb1b/attachment.sig>

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2020-04-17 21:09 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-26  5:35 [PATCH U-BOOT v2 0/3] fs: btrfs: Fix false LZO decompression error due to missing page boundary check Qu Wenruo
2020-03-26  5:35 ` Qu Wenruo
2020-03-26  5:35 ` [PATCH U-BOOT v2 1/3] fs: btrfs: Use LZO_LEN to replace immediate number Qu Wenruo
2020-03-26  5:35   ` Qu Wenruo
2020-03-26  9:02   ` Marek Behun
2020-03-26  9:02     ` Marek Behun
2020-04-17 21:08   ` Tom Rini
2020-04-17 21:08     ` Tom Rini
2020-03-26  5:35 ` [PATCH U-BOOT v2 2/3] fs: btrfs: Reject fs with sector size other than PAGE_SIZE Qu Wenruo
2020-03-26  5:35   ` Qu Wenruo
2020-03-26  9:03   ` Marek Behun
2020-03-26  9:03     ` Marek Behun
2020-04-17 21:09   ` Tom Rini
2020-04-17 21:09     ` Tom Rini
2020-03-26  5:35 ` [PATCH U-BOOT v2 3/3] fs: btrfs: Fix LZO false decompression error caused by pending zero Qu Wenruo
2020-03-26  5:35   ` Qu Wenruo
2020-03-26  9:03   ` Marek Behun
2020-03-26  9:03     ` Marek Behun
2020-04-17 21:09   ` Tom Rini
2020-04-17 21:09     ` Tom Rini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.