* [PATCH] lz4: fixs use-after-free Read in LZ4_decompress_safe_partial
@ 2021-06-30 3:23 Chengyang Fan
2021-06-30 11:42 ` Gao Xiang
0 siblings, 1 reply; 5+ messages in thread
From: Chengyang Fan @ 2021-06-30 3:23 UTC (permalink / raw)
To: akpm, terrelln, sfr, hsiangkao, thisisrast7; +Cc: linux-kernel, cy.fan
==================================================================
BUG: KASAN: use-after-free in get_unaligned_le16 include/linux/unaligned/access_ok.h:10 [inline]
BUG: KASAN: use-after-free in LZ4_readLE16 lib/lz4/lz4defs.h:132 [inline]
BUG: KASAN: use-after-free in LZ4_decompress_generic lib/lz4/lz4_decompress.c:281 [inline]
BUG: KASAN: use-after-free in LZ4_decompress_safe_partial+0xf50/0x1480 lib/lz4/lz4_decompress.c:465
Read of size 2 at addr ffff888017851000 by task kworker/u12:0/2056
CPU: 0 PID: 2056 Comm: kworker/u12:0 Not tainted 5.10.40 #2
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
Workqueue: erofs_unzipd z_erofs_decompressqueue_work
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x137/0x1be lib/dump_stack.c:118
print_address_description+0x6c/0x640 mm/kasan/report.c:385
__kasan_report mm/kasan/report.c:545 [inline]
kasan_report+0x13d/0x1e0 mm/kasan/report.c:562
get_unaligned_le16 include/linux/unaligned/access_ok.h:10 [inline]
LZ4_readLE16 lib/lz4/lz4defs.h:132 [inline]
LZ4_decompress_generic lib/lz4/lz4_decompress.c:281 [inline]
LZ4_decompress_safe_partial+0xf50/0x1480 lib/lz4/lz4_decompress.c:465
z_erofs_lz4_decompress+0x839/0xc90 fs/erofs/decompressor.c:162
z_erofs_decompress_generic fs/erofs/decompressor.c:291 [inline]
z_erofs_decompress+0x57e/0xe10 fs/erofs/decompressor.c:344
z_erofs_decompress_pcluster+0x13d1/0x2310 fs/erofs/zdata.c:880
z_erofs_decompress_queue fs/erofs/zdata.c:958 [inline]
z_erofs_decompressqueue_work+0xde/0x140 fs/erofs/zdata.c:969
process_one_work+0x780/0xfc0 kernel/workqueue.c:2269
worker_thread+0xaa4/0x1460 kernel/workqueue.c:2415
kthread+0x39a/0x3c0 kernel/kthread.c:292
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:296
The buggy address belongs to the page:
page:00000000a79b76f1 refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x17851
flags: 0xfff00000000000()
raw: 00fff00000000000 ffffea000081b9c8 ffffea00006ac6c8 0000000000000000
raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff888017850f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff888017850f80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>ffff888017851000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
^
ffff888017851080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ffff888017851100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
==================================================================
erofs: (device loop0): z_erofs_lz4_decompress: failed to decompress -4099 in[4096, 0] out[9000]
Off-by-one error causes the above issue. In LZ4_decompress_generic(),
`iend = src + srcSize`. It means the valid address range should be
[src, iend - 1]. Therefore, when checking whether the reading is
out-of-bounds, it should be `>= iend` rather than `> iend`.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Chengyang Fan <cy.fan@huawei.com>
---
lib/lz4/lz4_decompress.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/lz4/lz4_decompress.c b/lib/lz4/lz4_decompress.c
index 926f4823d5ea..ec51837cd31f 100644
--- a/lib/lz4/lz4_decompress.c
+++ b/lib/lz4/lz4_decompress.c
@@ -234,7 +234,7 @@ static FORCE_INLINE int LZ4_decompress_generic(
length = oend - op;
}
if ((endOnInput)
- && (ip + length > iend)) {
+ && (ip + length >= iend)) {
/*
* Error :
* read attempt beyond
--
2.18.0.huawei.25
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] lz4: fixs use-after-free Read in LZ4_decompress_safe_partial
2021-06-30 3:23 [PATCH] lz4: fixs use-after-free Read in LZ4_decompress_safe_partial Chengyang Fan
@ 2021-06-30 11:42 ` Gao Xiang
2021-06-30 17:49 ` Nick Terrell
2021-07-01 2:32 ` Chengyang Fan
0 siblings, 2 replies; 5+ messages in thread
From: Gao Xiang @ 2021-06-30 11:42 UTC (permalink / raw)
To: Chengyang Fan
Cc: akpm, terrelln, sfr, thisisrast7, linux-kernel, Yann Collet, linux-erofs
(also +cc Yann as well as Nick..)
Hi Chengyang,
If I understand correctly, is this a manually produced fuzzed
EROFS compressed data? If it's just a normal image, could you
also share the original image?
On Wed, Jun 30, 2021 at 11:23:58AM +0800, Chengyang Fan wrote:
> ==================================================================
> BUG: KASAN: use-after-free in get_unaligned_le16 include/linux/unaligned/access_ok.h:10 [inline]
> BUG: KASAN: use-after-free in LZ4_readLE16 lib/lz4/lz4defs.h:132 [inline]
> BUG: KASAN: use-after-free in LZ4_decompress_generic lib/lz4/lz4_decompress.c:281 [inline]
> BUG: KASAN: use-after-free in LZ4_decompress_safe_partial+0xf50/0x1480 lib/lz4/lz4_decompress.c:465
> Read of size 2 at addr ffff888017851000 by task kworker/u12:0/2056
>
> CPU: 0 PID: 2056 Comm: kworker/u12:0 Not tainted 5.10.40 #2
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
> Workqueue: erofs_unzipd z_erofs_decompressqueue_work
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x137/0x1be lib/dump_stack.c:118
> print_address_description+0x6c/0x640 mm/kasan/report.c:385
> __kasan_report mm/kasan/report.c:545 [inline]
> kasan_report+0x13d/0x1e0 mm/kasan/report.c:562
> get_unaligned_le16 include/linux/unaligned/access_ok.h:10 [inline]
> LZ4_readLE16 lib/lz4/lz4defs.h:132 [inline]
> LZ4_decompress_generic lib/lz4/lz4_decompress.c:281 [inline]
> LZ4_decompress_safe_partial+0xf50/0x1480 lib/lz4/lz4_decompress.c:465
> z_erofs_lz4_decompress+0x839/0xc90 fs/erofs/decompressor.c:162
> z_erofs_decompress_generic fs/erofs/decompressor.c:291 [inline]
> z_erofs_decompress+0x57e/0xe10 fs/erofs/decompressor.c:344
> z_erofs_decompress_pcluster+0x13d1/0x2310 fs/erofs/zdata.c:880
> z_erofs_decompress_queue fs/erofs/zdata.c:958 [inline]
> z_erofs_decompressqueue_work+0xde/0x140 fs/erofs/zdata.c:969
> process_one_work+0x780/0xfc0 kernel/workqueue.c:2269
> worker_thread+0xaa4/0x1460 kernel/workqueue.c:2415
> kthread+0x39a/0x3c0 kernel/kthread.c:292
> ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:296
>
> The buggy address belongs to the page:
> page:00000000a79b76f1 refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x17851
> flags: 0xfff00000000000()
> raw: 00fff00000000000 ffffea000081b9c8 ffffea00006ac6c8 0000000000000000
> raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> ffff888017850f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffff888017850f80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >ffff888017851000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ^
> ffff888017851080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ffff888017851100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ==================================================================
> erofs: (device loop0): z_erofs_lz4_decompress: failed to decompress -4099 in[4096, 0] out[9000]
>
> Off-by-one error causes the above issue. In LZ4_decompress_generic(),
> `iend = src + srcSize`. It means the valid address range should be
> [src, iend - 1]. Therefore, when checking whether the reading is
> out-of-bounds, it should be `>= iend` rather than `> iend`.
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Chengyang Fan <cy.fan@huawei.com>
> ---
> lib/lz4/lz4_decompress.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/lz4/lz4_decompress.c b/lib/lz4/lz4_decompress.c
> index 926f4823d5ea..ec51837cd31f 100644
> --- a/lib/lz4/lz4_decompress.c
> +++ b/lib/lz4/lz4_decompress.c
> @@ -234,7 +234,7 @@ static FORCE_INLINE int LZ4_decompress_generic(
> length = oend - op;
> }
> if ((endOnInput)
> - && (ip + length > iend)) {
> + && (ip + length >= iend)) {
I'm not sure it should be fixed as this.
The current lz4 decompression code was from lz4 1.8.3, and I saw
several following up fixes for incomplete input partial decoding
in recent LZ4 upstream, you could check them out together. However,
EROFS should never pass incomplete lz4 compressed data to the LZ4
side unless it's somewhat a corrupted image on purpose.
https://github.com/lz4/lz4/blame/dev/lib/lz4.c
Thanks,
Gao Xiang
> /*
> * Error :
> * read attempt beyond
> --
> 2.18.0.huawei.25
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] lz4: fixs use-after-free Read in LZ4_decompress_safe_partial
2021-06-30 11:42 ` Gao Xiang
@ 2021-06-30 17:49 ` Nick Terrell
2021-07-01 2:34 ` Chengyang Fan
2021-07-01 2:32 ` Chengyang Fan
1 sibling, 1 reply; 5+ messages in thread
From: Nick Terrell @ 2021-06-30 17:49 UTC (permalink / raw)
To: Gao Xiang
Cc: Chengyang Fan, Andrew Morton, Stephen Rothwell, Rajat Asthana,
LKML, Yann Collet, linux-erofs
> On Jun 30, 2021, at 4:42 AM, Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
>
> (also +cc Yann as well as Nick..)
>
> Hi Chengyang,
>
> If I understand correctly, is this a manually produced fuzzed
> EROFS compressed data? If it's just a normal image, could you
> also share the original image?
>
> On Wed, Jun 30, 2021 at 11:23:58AM +0800, Chengyang Fan wrote:
>> ==================================================================
>> BUG: KASAN: use-after-free in get_unaligned_le16 include/linux/unaligned/access_ok.h:10 [inline]
>> BUG: KASAN: use-after-free in LZ4_readLE16 lib/lz4/lz4defs.h:132 [inline]
>> BUG: KASAN: use-after-free in LZ4_decompress_generic lib/lz4/lz4_decompress.c:281 [inline]
>> BUG: KASAN: use-after-free in LZ4_decompress_safe_partial+0xf50/0x1480 lib/lz4/lz4_decompress.c:465
>> Read of size 2 at addr ffff888017851000 by task kworker/u12:0/2056
>>
>> CPU: 0 PID: 2056 Comm: kworker/u12:0 Not tainted 5.10.40 #2
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
>> Workqueue: erofs_unzipd z_erofs_decompressqueue_work
>> Call Trace:
>> __dump_stack lib/dump_stack.c:77 [inline]
>> dump_stack+0x137/0x1be lib/dump_stack.c:118
>> print_address_description+0x6c/0x640 mm/kasan/report.c:385
>> __kasan_report mm/kasan/report.c:545 [inline]
>> kasan_report+0x13d/0x1e0 mm/kasan/report.c:562
>> get_unaligned_le16 include/linux/unaligned/access_ok.h:10 [inline]
>> LZ4_readLE16 lib/lz4/lz4defs.h:132 [inline]
>> LZ4_decompress_generic lib/lz4/lz4_decompress.c:281 [inline]
>> LZ4_decompress_safe_partial+0xf50/0x1480 lib/lz4/lz4_decompress.c:465
>> z_erofs_lz4_decompress+0x839/0xc90 fs/erofs/decompressor.c:162
>> z_erofs_decompress_generic fs/erofs/decompressor.c:291 [inline]
>> z_erofs_decompress+0x57e/0xe10 fs/erofs/decompressor.c:344
>> z_erofs_decompress_pcluster+0x13d1/0x2310 fs/erofs/zdata.c:880
>> z_erofs_decompress_queue fs/erofs/zdata.c:958 [inline]
>> z_erofs_decompressqueue_work+0xde/0x140 fs/erofs/zdata.c:969
>> process_one_work+0x780/0xfc0 kernel/workqueue.c:2269
>> worker_thread+0xaa4/0x1460 kernel/workqueue.c:2415
>> kthread+0x39a/0x3c0 kernel/kthread.c:292
>> ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:296
>>
>> The buggy address belongs to the page:
>> page:00000000a79b76f1 refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x17851
>> flags: 0xfff00000000000()
>> raw: 00fff00000000000 ffffea000081b9c8 ffffea00006ac6c8 0000000000000000
>> raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
>> page dumped because: kasan: bad access detected
>>
>> Memory state around the buggy address:
>> ffff888017850f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> ffff888017850f80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> ffff888017851000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>> ^
>> ffff888017851080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>> ffff888017851100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>> ==================================================================
>> erofs: (device loop0): z_erofs_lz4_decompress: failed to decompress -4099 in[4096, 0] out[9000]
>>
>> Off-by-one error causes the above issue. In LZ4_decompress_generic(),
>> `iend = src + srcSize`. It means the valid address range should be
>> [src, iend - 1]. Therefore, when checking whether the reading is
>> out-of-bounds, it should be `>= iend` rather than `> iend`.
This isn’t correct. The bounds check is correct for the following memcpy().
The problem is the LZ4_readLE16() on line 285 [1].
>> Reported-by: Hulk Robot <hulkci@huawei.com>
>> Signed-off-by: Chengyang Fan <cy.fan@huawei.com>
>> ---
>> lib/lz4/lz4_decompress.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/lz4/lz4_decompress.c b/lib/lz4/lz4_decompress.c
>> index 926f4823d5ea..ec51837cd31f 100644
>> --- a/lib/lz4/lz4_decompress.c
>> +++ b/lib/lz4/lz4_decompress.c
>> @@ -234,7 +234,7 @@ static FORCE_INLINE int LZ4_decompress_generic(
>> length = oend - op;
>> }
>> if ((endOnInput)
>> - && (ip + length > iend)) {
>> + && (ip + length >= iend)) {
>
> I'm not sure it should be fixed as this.
Yeah, this is not the correct fix. `ip + length > iend` is correct for the
memcpy(). You would instead need to add another check after the
memcpy(). E.g. something like this, taken from upstream [2].
```
LZ4_memmove(op, ip, length);
ip += length;
op += length;
/* Necessarily EOF, due to parsing restrictions */
- if (!partialDecoding || (cpy == oend))
+ if (!partialDecoding || (cpy == oend) || (ip >= (iend-2)))
break;
```
This should be enough to fix the out of bounds read you reported.
However, I can’t guarantee that this will fix all the issues that have been
fixed upstream since v1.8.3.
The safest and most robust course of action would be to update the version
of LZ4 in the kernel to the latest upstream, which is continuously fuzzed, and
doesn't have this issue.
Best,
Nick Terrell
> The current lz4 decompression code was from lz4 1.8.3, and I saw
> several following up fixes for incomplete input partial decoding
> in recent LZ4 upstream, you could check them out together. However,
> EROFS should never pass incomplete lz4 compressed data to the LZ4
> side unless it's somewhat a corrupted image on purpose.
> https://github.com/lz4/lz4/blame/dev/lib/lz4.c
>
> Thanks,
> Gao Xiang
>
>> /*
>> * Error :
>> * read attempt beyond
>> --
>> 2.18.0.huawei.25
[1] https://github.com/torvalds/linux/blob/007b350a58754a93ca9fe50c498cc27780171153/lib/lz4/lz4_decompress.c#L285
[2] https://github.com/lz4/lz4/blob/11efc95c3f02b76ab40e7fbd605677ad6eb141d3/lib/lz4.c#L2059
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] lz4: fixs use-after-free Read in LZ4_decompress_safe_partial
2021-06-30 11:42 ` Gao Xiang
2021-06-30 17:49 ` Nick Terrell
@ 2021-07-01 2:32 ` Chengyang Fan
1 sibling, 0 replies; 5+ messages in thread
From: Chengyang Fan @ 2021-07-01 2:32 UTC (permalink / raw)
To: Gao Xiang
Cc: akpm, terrelln, sfr, thisisrast7, linux-kernel, Yann Collet, linux-erofs
On 2021/6/30 19:42, Gao Xiang wrote:
> (also +cc Yann as well as Nick..)
>
> Hi Chengyang,
>
> If I understand correctly, is this a manually produced fuzzed
> EROFS compressed data? If it's just a normal image, could you
> also share the original image?
Yeah, this is the generated fuzzed data, not the normal image.
> On Wed, Jun 30, 2021 at 11:23:58AM +0800, Chengyang Fan wrote:
>> ==================================================================
>> BUG: KASAN: use-after-free in get_unaligned_le16 include/linux/unaligned/access_ok.h:10 [inline]
>> BUG: KASAN: use-after-free in LZ4_readLE16 lib/lz4/lz4defs.h:132 [inline]
>> BUG: KASAN: use-after-free in LZ4_decompress_generic lib/lz4/lz4_decompress.c:281 [inline]
>> BUG: KASAN: use-after-free in LZ4_decompress_safe_partial+0xf50/0x1480 lib/lz4/lz4_decompress.c:465
>> Read of size 2 at addr ffff888017851000 by task kworker/u12:0/2056
>>
>> CPU: 0 PID: 2056 Comm: kworker/u12:0 Not tainted 5.10.40 #2
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
>> Workqueue: erofs_unzipd z_erofs_decompressqueue_work
>> Call Trace:
>> __dump_stack lib/dump_stack.c:77 [inline]
>> dump_stack+0x137/0x1be lib/dump_stack.c:118
>> print_address_description+0x6c/0x640 mm/kasan/report.c:385
>> __kasan_report mm/kasan/report.c:545 [inline]
>> kasan_report+0x13d/0x1e0 mm/kasan/report.c:562
>> get_unaligned_le16 include/linux/unaligned/access_ok.h:10 [inline]
>> LZ4_readLE16 lib/lz4/lz4defs.h:132 [inline]
>> LZ4_decompress_generic lib/lz4/lz4_decompress.c:281 [inline]
>> LZ4_decompress_safe_partial+0xf50/0x1480 lib/lz4/lz4_decompress.c:465
>> z_erofs_lz4_decompress+0x839/0xc90 fs/erofs/decompressor.c:162
>> z_erofs_decompress_generic fs/erofs/decompressor.c:291 [inline]
>> z_erofs_decompress+0x57e/0xe10 fs/erofs/decompressor.c:344
>> z_erofs_decompress_pcluster+0x13d1/0x2310 fs/erofs/zdata.c:880
>> z_erofs_decompress_queue fs/erofs/zdata.c:958 [inline]
>> z_erofs_decompressqueue_work+0xde/0x140 fs/erofs/zdata.c:969
>> process_one_work+0x780/0xfc0 kernel/workqueue.c:2269
>> worker_thread+0xaa4/0x1460 kernel/workqueue.c:2415
>> kthread+0x39a/0x3c0 kernel/kthread.c:292
>> ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:296
>>
>> The buggy address belongs to the page:
>> page:00000000a79b76f1 refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x17851
>> flags: 0xfff00000000000()
>> raw: 00fff00000000000 ffffea000081b9c8 ffffea00006ac6c8 0000000000000000
>> raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
>> page dumped because: kasan: bad access detected
>>
>> Memory state around the buggy address:
>> ffff888017850f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> ffff888017850f80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> ffff888017851000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>> ^
>> ffff888017851080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>> ffff888017851100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>> ==================================================================
>> erofs: (device loop0): z_erofs_lz4_decompress: failed to decompress -4099 in[4096, 0] out[9000]
>>
>> Off-by-one error causes the above issue. In LZ4_decompress_generic(),
>> `iend = src + srcSize`. It means the valid address range should be
>> [src, iend - 1]. Therefore, when checking whether the reading is
>> out-of-bounds, it should be `>= iend` rather than `> iend`.
>>
>> Reported-by: Hulk Robot <hulkci@huawei.com>
>> Signed-off-by: Chengyang Fan <cy.fan@huawei.com>
>> ---
>> lib/lz4/lz4_decompress.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/lz4/lz4_decompress.c b/lib/lz4/lz4_decompress.c
>> index 926f4823d5ea..ec51837cd31f 100644
>> --- a/lib/lz4/lz4_decompress.c
>> +++ b/lib/lz4/lz4_decompress.c
>> @@ -234,7 +234,7 @@ static FORCE_INLINE int LZ4_decompress_generic(
>> length = oend - op;
>> }
>> if ((endOnInput)
>> - && (ip + length > iend)) {
>> + && (ip + length >= iend)) {
> I'm not sure it should be fixed as this.
>
> The current lz4 decompression code was from lz4 1.8.3, and I saw
> several following up fixes for incomplete input partial decoding
> in recent LZ4 upstream, you could check them out together. However,
> EROFS should never pass incomplete lz4 compressed data to the LZ4
> side unless it's somewhat a corrupted image on purpose.
> https://github.com/lz4/lz4/blame/dev/lib/lz4.c
>
> Thanks,
> Gao Xiang
This is the fault injection, not an error caused by normal EROFS.
Thanks for you advice, I will recheck the bugfixes of LZ4 upstream.
Thanks,
Chengyang Fan
>> /*
>> * Error :
>> * read attempt beyond
>> --
>> 2.18.0.huawei.25
> .
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] lz4: fixs use-after-free Read in LZ4_decompress_safe_partial
2021-06-30 17:49 ` Nick Terrell
@ 2021-07-01 2:34 ` Chengyang Fan
0 siblings, 0 replies; 5+ messages in thread
From: Chengyang Fan @ 2021-07-01 2:34 UTC (permalink / raw)
To: Nick Terrell, Gao Xiang
Cc: Andrew Morton, Stephen Rothwell, Rajat Asthana, LKML,
Yann Collet, linux-erofs
On 2021/7/1 1:49, Nick Terrell wrote:
>
>> On Jun 30, 2021, at 4:42 AM, Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
>>
>> (also +cc Yann as well as Nick..)
>>
>> Hi Chengyang,
>>
>> If I understand correctly, is this a manually produced fuzzed
>> EROFS compressed data? If it's just a normal image, could you
>> also share the original image?
>>
>> On Wed, Jun 30, 2021 at 11:23:58AM +0800, Chengyang Fan wrote:
>>> ==================================================================
>>> BUG: KASAN: use-after-free in get_unaligned_le16 include/linux/unaligned/access_ok.h:10 [inline]
>>> BUG: KASAN: use-after-free in LZ4_readLE16 lib/lz4/lz4defs.h:132 [inline]
>>> BUG: KASAN: use-after-free in LZ4_decompress_generic lib/lz4/lz4_decompress.c:281 [inline]
>>> BUG: KASAN: use-after-free in LZ4_decompress_safe_partial+0xf50/0x1480 lib/lz4/lz4_decompress.c:465
>>> Read of size 2 at addr ffff888017851000 by task kworker/u12:0/2056
>>>
>>> CPU: 0 PID: 2056 Comm: kworker/u12:0 Not tainted 5.10.40 #2
>>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
>>> Workqueue: erofs_unzipd z_erofs_decompressqueue_work
>>> Call Trace:
>>> __dump_stack lib/dump_stack.c:77 [inline]
>>> dump_stack+0x137/0x1be lib/dump_stack.c:118
>>> print_address_description+0x6c/0x640 mm/kasan/report.c:385
>>> __kasan_report mm/kasan/report.c:545 [inline]
>>> kasan_report+0x13d/0x1e0 mm/kasan/report.c:562
>>> get_unaligned_le16 include/linux/unaligned/access_ok.h:10 [inline]
>>> LZ4_readLE16 lib/lz4/lz4defs.h:132 [inline]
>>> LZ4_decompress_generic lib/lz4/lz4_decompress.c:281 [inline]
>>> LZ4_decompress_safe_partial+0xf50/0x1480 lib/lz4/lz4_decompress.c:465
>>> z_erofs_lz4_decompress+0x839/0xc90 fs/erofs/decompressor.c:162
>>> z_erofs_decompress_generic fs/erofs/decompressor.c:291 [inline]
>>> z_erofs_decompress+0x57e/0xe10 fs/erofs/decompressor.c:344
>>> z_erofs_decompress_pcluster+0x13d1/0x2310 fs/erofs/zdata.c:880
>>> z_erofs_decompress_queue fs/erofs/zdata.c:958 [inline]
>>> z_erofs_decompressqueue_work+0xde/0x140 fs/erofs/zdata.c:969
>>> process_one_work+0x780/0xfc0 kernel/workqueue.c:2269
>>> worker_thread+0xaa4/0x1460 kernel/workqueue.c:2415
>>> kthread+0x39a/0x3c0 kernel/kthread.c:292
>>> ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:296
>>>
>>> The buggy address belongs to the page:
>>> page:00000000a79b76f1 refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x17851
>>> flags: 0xfff00000000000()
>>> raw: 00fff00000000000 ffffea000081b9c8 ffffea00006ac6c8 0000000000000000
>>> raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
>>> page dumped because: kasan: bad access detected
>>>
>>> Memory state around the buggy address:
>>> ffff888017850f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> ffff888017850f80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>>> ffff888017851000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>>> ^
>>> ffff888017851080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>>> ffff888017851100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>>> ==================================================================
>>> erofs: (device loop0): z_erofs_lz4_decompress: failed to decompress -4099 in[4096, 0] out[9000]
>>>
>>> Off-by-one error causes the above issue. In LZ4_decompress_generic(),
>>> `iend = src + srcSize`. It means the valid address range should be
>>> [src, iend - 1]. Therefore, when checking whether the reading is
>>> out-of-bounds, it should be `>= iend` rather than `> iend`.
> This isn’t correct. The bounds check is correct for the following memcpy().
> The problem is the LZ4_readLE16() on line 285 [1].
Yeah, it's my fault. This fix is incorrect.
>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>> Signed-off-by: Chengyang Fan <cy.fan@huawei.com>
>>> ---
>>> lib/lz4/lz4_decompress.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/lib/lz4/lz4_decompress.c b/lib/lz4/lz4_decompress.c
>>> index 926f4823d5ea..ec51837cd31f 100644
>>> --- a/lib/lz4/lz4_decompress.c
>>> +++ b/lib/lz4/lz4_decompress.c
>>> @@ -234,7 +234,7 @@ static FORCE_INLINE int LZ4_decompress_generic(
>>> length = oend - op;
>>> }
>>> if ((endOnInput)
>>> - && (ip + length > iend)) {
>>> + && (ip + length >= iend)) {
>> I'm not sure it should be fixed as this.
> Yeah, this is not the correct fix. `ip + length > iend` is correct for the
> memcpy(). You would instead need to add another check after the
> memcpy(). E.g. something like this, taken from upstream [2].
>
> ```
>
> LZ4_memmove(op, ip, length);
> ip += length;
> op += length;
>
> /* Necessarily EOF, due to parsing restrictions */
> - if (!partialDecoding || (cpy == oend))
> + if (!partialDecoding || (cpy == oend) || (ip >= (iend-2)))
> break;
> ```
>
> This should be enough to fix the out of bounds read you reported.
> However, I can’t guarantee that this will fix all the issues that have been
> fixed upstream since v1.8.3.
>
> The safest and most robust course of action would be to update the version
> of LZ4 in the kernel to the latest upstream, which is continuously fuzzed, and
> doesn't have this issue.
>
> Best,
> Nick Terrell
Thanks for pointing this out. I ignored the modifications of LZ4
upstream before. So I'll
recheck them and find the right way to fix this error.
Thanks,
Chengyang Fan
>> The current lz4 decompression code was from lz4 1.8.3, and I saw
>> several following up fixes for incomplete input partial decoding
>> in recent LZ4 upstream, you could check them out together. However,
>> EROFS should never pass incomplete lz4 compressed data to the LZ4
>> side unless it's somewhat a corrupted image on purpose.
>> https://github.com/lz4/lz4/blame/dev/lib/lz4.c
>>
>> Thanks,
>> Gao Xiang
>>
>>> /*
>>> * Error :
>>> * read attempt beyond
>>> --
>>> 2.18.0.huawei.25
> [1] https://github.com/torvalds/linux/blob/007b350a58754a93ca9fe50c498cc27780171153/lib/lz4/lz4_decompress.c#L285
> [2] https://github.com/lz4/lz4/blob/11efc95c3f02b76ab40e7fbd605677ad6eb141d3/lib/lz4.c#L2059
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-07-01 2:34 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-30 3:23 [PATCH] lz4: fixs use-after-free Read in LZ4_decompress_safe_partial Chengyang Fan
2021-06-30 11:42 ` Gao Xiang
2021-06-30 17:49 ` Nick Terrell
2021-07-01 2:34 ` Chengyang Fan
2021-07-01 2:32 ` Chengyang Fan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).