Re: Several potential vulnerabilities in the filesystem
On 2024/6/5 19:18, Gao Xiang wrote:
> Hi Jianqiang,
>
> On 2024/6/5 19:00, jianqiang wang wrote:
>> Hi,
>>
>> I do have the crafted image.
>>
>> payload_00500, payload_00763, payload_00846 can be used to reproduce
>> 1,2,3 vulnerabilities respectively.
>>
>> Each image is a hard drive file and the vulnerabilities can be
>> triggered by performing the following operations:
>>
>> struct udevice *dev;
>> uclass_first_device_err(UCLASS_IDE, ); //detect the block
>> device
>>
>> fs_set_blk_dev("ide","0:1", 0);
>> fs_ls("/"); //mount the first partition and list the root
>> directory files
>>
>> fs_set_blk_dev("ide","0:1", 0);
>> char buf[10];
>> buf[0] = 0;
>> buf[1] = 0;
>> buf[2] = 0;
>> buf[3] = 0;
>> loff_t actread = 0;
>> fs_read("/a.txt", (ulong)buf, 0, 5, );
>> printf("fd actread %lld %x %x %x\n",actread,buf[0],buf[1],buf[2]);
>> read the /a.txt file
>>
>>
>> fs_set_blk_dev("ide","0:1", 0);
>> fs_read("/a.txt.ln", (ulong)buf, 0, 5, );
>> printf("fd actread %lld %x %x %x\n",actread,buf[0],buf[1],buf[2]);
>> read the /a.txt.ln symbol file
>>
>> fs_set_blk_dev("ide","0:1", 0);
>> fs_unlink("/a.txt.ln"); //unlink it
>>
>> The second and third may not trigger a crash however, can be observed
>> by inserting logging before the memset/memcpy function.
>
> Sorry, I just found that this issue was already fixed in erofs-utils:
> https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git/commit/?id=884866ca07817e97c59605a2fa858a0b732d3f3c
>
>
> Would you mind checking if the patch above fixes the issue?
>
> Hi Jianan,
> Would you mind backporting this patch for u-boot?
https://lore.kernel.org/u-boot/20240605140554.1883-1-jnhuan...@gmail.com/T/#u
Thanks,
Jianan
>
> Thanks,
> Gao Xiang
>
>>
>> Best regards
>>
>> Gao Xiang 于2024年6月5日周三 05:10写道：
>>>
>>>
>>>
>>> On 2024/6/5 06:53, jianqiang wang wrote:
Hi Das U-Boot developers,
>>>
>>> ...
>>>
2. in file fs/erofs/data.c, function z_erofs_read_one_data, the node
data is read from the storage, however, without a proper check, the
data can be corrupted. For example, the inode data is used in function
z_erofs_read_data, map.m_llen will be calculated to a very large
value, which means the length variable will be very large. It will
cause a large memory clear with memset(buffer + end - offset, 0,
length);
>>>
>>> Would you mind giving a reproducer or a crafted image to trigger
>>> this? Or it's your pure observation.
>>>
>>> Thanks,
>>> Gao XIang
>>>
[PATCH] fs/erofs: fix an overflow issue of unmapped extents
Here the size should be `length - skip`, otherwise it could cause
the destination buffer overflow.
Reported-by: jianqiang wang
Fixes: 65cb73057b65 ("fs/erofs: add lz4 decompression support")
Signed-off-by: Jianan Huang
---
fs/erofs/data.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/erofs/data.c b/fs/erofs/data.c
index f4b21d7917..95b609d8ea 100644
--- a/fs/erofs/data.c
+++ b/fs/erofs/data.c
@@ -313,7 +313,7 @@ static int z_erofs_read_data(struct erofs_inode *inode,
char *buffer,
}
if (!(map.m_flags & EROFS_MAP_MAPPED)) {
- memset(buffer + end - offset, 0, length);
+ memset(buffer + end - offset, 0, length - skip);
end = map.m_la;
continue;
}
--
2.34.1
Re: [PATCH v4 2/5] lib/lz4: update LZ4 decompressor module
Hi Jonathan,
Could you please try the following patch ? It replaces all memcpy() calls
in lz4 with __builtin_memcpy().
diff --git a/lib/lz4.c b/lib/lz4.c
index d365dc727c..2afe31c1c3 100644
--- a/lib/lz4.c
+++ b/lib/lz4.c
@@ -34,6 +34,8 @@
#include
#include
+#define LZ4_memcpy(dst, src, size) __builtin_memcpy(dst, src, size)
+
#define FORCE_INLINE inline __attribute__((always_inline))
static FORCE_INLINE u16 LZ4_readLE16(const void *src)
@@ -215,7 +217,7 @@ static FORCE_INLINE int LZ4_decompress_generic(
&& likely((endOnInput ? ip < shortiend : 1) &
(op <= shortoend))) {
/* Copy the literals */
- memcpy(op, ip, endOnInput ? 16 : 8);
+ LZ4_memcpy(op, ip, endOnInput ? 16 : 8);
op += length; ip += length;
/*
@@ -234,9 +236,9 @@ static FORCE_INLINE int LZ4_decompress_generic(
(offset >= 8) &&
(dict == withPrefix64k || match >= lowPrefix)) {
/* Copy the match. */
- memcpy(op + 0, match + 0, 8);
- memcpy(op + 8, match + 8, 8);
- memcpy(op + 16, match + 16, 2);
+ LZ4_memcpy(op + 0, match + 0, 8);
+ LZ4_memcpy(op + 8, match + 8, 8);
+ LZ4_memcpy(op + 16, match + 16, 2);
op += length + MINMATCH;
/* Both stages worked, load the next token. */
continue;
@@ -416,7 +418,7 @@ _copy_match:
size_t const copySize = (size_t)(lowPrefix - match);
size_t const restSize = length - copySize;
- memcpy(op, dictEnd - copySize, copySize);
+ LZ4_memcpy(op, dictEnd - copySize, copySize);
op += copySize;
if (restSize > (size_t)(op - lowPrefix)) {
/* overlap copy */
@@ -426,7 +428,7 @@ _copy_match:
while (op < endOfMatch)
*op++ = *copyFrom++;
} else {
- memcpy(op, lowPrefix, restSize);
+ LZ4_memcpy(op, lowPrefix, restSize);
op += restSize;
}
}
@@ -452,7 +454,7 @@ _copy_match:
while (op < copyEnd)
*op++ = *match++;
} else {
- memcpy(op, match, mlen);
+ LZ4_memcpy(op, match, mlen);
}
op = copyEnd;
if (op == oend)
@@ -466,7 +468,7 @@ _copy_match:
op[2] = match[2];
op[3] = match[3];
match += inc32table[offset];
- memcpy(op + 4, match, 4);
+ LZ4_memcpy(op + 4, match, 4);
match -= dec64table[offset];
} else {
LZ4_copy8(op, match);
diff --git a/lib/lz4_wrapper.c b/lib/lz4_wrapper.c
index 4d48e7b0e8..e09c8d7057 100644
--- a/lib/lz4_wrapper.c
+++ b/lib/lz4_wrapper.c
@@ -80,7 +80,7 @@ int ulz4fn(const void *src, size_t srcn, void *dst,
size_t *dstn)
if (block_header & LZ4F_BLOCKUNCOMPRESSED_FLAG) {
size_t size = min((ptrdiff_t)block_size, (ptrdiff_t)(end - out));
- memcpy(out, in, size);
+ LZ4_memcpy(out, in, size);
out += size;
if (size < block_size) {
ret = -ENOBUFS; /* output overrun */
Thanks,
Jianan
On 2024/5/26 16:06, Jonathan Liu wrote:
Hi Gao,
On Sat, 25 May 2024 at 02:52, Gao Xiang
wrote:
Hi,
On 2024/5/24 22:26, Jonathan Liu wrote:
Hi Jianan,
On Sat, 26 Feb 2022 at 18:05, Huang Jianan
wrote:
Update the LZ4 compression module based on LZ4 v1.8.3 in order to
use the newest LZ4_decompress_safe_partial() which can now decode
exactly the nb of bytes requested.
Signed-off-by: Huang Jianan
I noticed after this commit LZ4 decompression is slower.
ulz4fn function call takes 1.209670 seconds with this commit.
After reverting this commit, the ulz4fn function call takes 0.587032 seconds.
I am decompressing a LZ4 compressed kernel (compressed with lz4 v1.9.4
using -9 option for maximum compression) on RK3399.
Any ideas why it is slower with this commit and how the performance
regression can be fixed?
Just the quick glance, I think the issue may be due to memcpy/memmove
since it seems the main difference between these two codebases
(I'm not sure which LZ4 version the old codebase was based on) and
the new version mainly relies on memcpy/memmove instead of its own
versions.
Would you mind to check the assembly how memcpy/memset is generated
on your platform?
Here is the assembly (-mcpu=cortex-a72.cortex-a53 -march=armv8-a+crc+crypto):
0028220c :
#if !CONFIG_IS_ENABLED(TINY_MEMSET)
unsigned long cl = 0;
int i;
/* do it one word at a time (32 bits or 64 bits) while possible */
if ( ((ulong)s & (sizeof(*sl) - 1)) == 0) {
28220c: f2400803andsx3, x0, #0x7
282210: 540002c1b.ne282268 // b.any
for (i = 0; i < sizeof(*sl); i++) {
cl <<= 8;
cl |= c & 0xff;
282214: 92401c26and x6, x1, #0xff
unsigned long cl = 0;
282218: d284mov x4, #0x0// #0
28221c: 52800105mov w5, #0x8// #8
cl |= c & 0xff;
282220: aa0420c4orr x4, x6, x4, lsl #8
for (i = 0; i < sizeof(*sl); i++) {
282224: 710004a5subsw5, w5, #0x1
282228: 54c1b.ne282220 // b.any
}
while (count >= sizeof(*sl)) {
28222c: cb030045sub x5, x2, x3
282230: f1001cbf
[PATCH] fs/erofs: add DEFLATE algorithm support
This patch adds DEFLATE compression algorithm support. It's a good choice
to trade off between compression ratios and performance compared to LZ4.
Alternatively, DEFLATE could be used for some specific files since EROFS
supports multiple compression algorithms in one image.
Signed-off-by: Jianan Huang
---
fs/erofs/Kconfig | 15
fs/erofs/decompress.c | 83 +++
fs/erofs/erofs_fs.h | 1 +
3 files changed, 99 insertions(+)
diff --git a/fs/erofs/Kconfig b/fs/erofs/Kconfig
index ee4e777c5c..c8463357ca 100644
--- a/fs/erofs/Kconfig
+++ b/fs/erofs/Kconfig
@@ -19,3 +19,18 @@ config FS_EROFS_ZIP
help
Enable fixed-sized output compression for EROFS.
If you don't want to enable compression feature, say N.
+
+config FS_EROFS_ZIP_DEFLATE
+ bool "EROFS DEFLATE compressed data support"
+ depends on FS_EROFS_ZIP
+ select ZLIB
+ help
+ Saying Y here includes support for reading EROFS file systems
+ containing DEFLATE compressed data. It gives better compression
+ ratios than the default LZ4 format, while it costs more CPU
+ overhead.
+
+ DEFLATE support is an experimental feature for now and so most
+ file systems will be readable without selecting this option.
+
+ If unsure, say N.
diff --git a/fs/erofs/decompress.c b/fs/erofs/decompress.c
index e04e5c34a8..ec74816534 100644
--- a/fs/erofs/decompress.c
+++ b/fs/erofs/decompress.c
@@ -1,6 +1,85 @@
// SPDX-License-Identifier: GPL-2.0+
#include "decompress.h"
+#if IS_ENABLED(CONFIG_ZLIB)
+#include
+
+/* report a zlib or i/o error */
+static int zerr(int ret)
+{
+ switch (ret) {
+ case Z_STREAM_ERROR:
+ return -EINVAL;
+ case Z_DATA_ERROR:
+ return -EIO;
+ case Z_MEM_ERROR:
+ return -ENOMEM;
+ case Z_ERRNO:
+ default:
+ return -EFAULT;
+ }
+}
+
+static int z_erofs_decompress_deflate(struct z_erofs_decompress_req *rq)
+{
+ u8 *dest = (u8 *)rq->out;
+ u8 *src = (u8 *)rq->in;
+ u8 *buff = NULL;
+ unsigned int inputmargin = 0;
+ z_stream strm;
+ int ret;
+
+ while (!src[inputmargin & (erofs_blksiz() - 1)])
+ if (!(++inputmargin & (erofs_blksiz() - 1)))
+ break;
+
+ if (inputmargin >= rq->inputsize)
+ return -EFSCORRUPTED;
+
+ if (rq->decodedskip) {
+ buff = malloc(rq->decodedlength);
+ if (!buff)
+ return -ENOMEM;
+ dest = buff;
+ }
+
+ /* allocate inflate state */
+ strm.zalloc = Z_NULL;
+ strm.zfree = Z_NULL;
+ strm.opaque = Z_NULL;
+ strm.avail_in = 0;
+ strm.next_in = Z_NULL;
+ ret = inflateInit2(, -15);
+ if (ret != Z_OK) {
+ free(buff);
+ return zerr(ret);
+ }
+
+ strm.next_in = src + inputmargin;
+ strm.avail_in = rq->inputsize - inputmargin;
+ strm.next_out = dest;
+ strm.avail_out = rq->decodedlength;
+
+ ret = inflate(, rq->partial_decoding ? Z_SYNC_FLUSH : Z_FINISH);
+ if (ret != Z_STREAM_END || strm.total_out != rq->decodedlength) {
+ if (ret != Z_OK || !rq->partial_decoding) {
+ ret = zerr(ret);
+ goto out_inflate_end;
+ }
+ }
+
+ if (rq->decodedskip)
+ memcpy(rq->out, dest + rq->decodedskip,
+ rq->decodedlength - rq->decodedskip);
+
+out_inflate_end:
+ inflateEnd();
+ if (buff)
+ free(buff);
+ return ret;
+}
+#endif
+
#if IS_ENABLED(CONFIG_LZ4)
#include
static int z_erofs_decompress_lz4(struct z_erofs_decompress_req *rq)
@@ -93,6 +172,10 @@ int z_erofs_decompress(struct z_erofs_decompress_req *rq)
#if IS_ENABLED(CONFIG_LZ4)
if (rq->alg == Z_EROFS_COMPRESSION_LZ4)
return z_erofs_decompress_lz4(rq);
+#endif
+#if IS_ENABLED(CONFIG_ZLIB)
+ if (rq->alg == Z_EROFS_COMPRESSION_DEFLATE)
+ return z_erofs_decompress_deflate(rq);
#endif
return -EOPNOTSUPP;
}
diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
index 158e2c68a1..5bac4fe1a1 100644
--- a/fs/erofs/erofs_fs.h
+++ b/fs/erofs/erofs_fs.h
@@ -304,6 +304,7 @@ enum {
enum {
Z_EROFS_COMPRESSION_LZ4 = 0,
Z_EROFS_COMPRESSION_LZMA= 1,
+ Z_EROFS_COMPRESSION_DEFLATE = 2,
Z_EROFS_COMPRESSION_MAX
};
--
2.34.1
