From: Edward Adam Davis via Linux-erofs <linux-erofs@lists.ozlabs.org>
To: hsiangkao@linux.alibaba.com
Cc: syzbot+6c746eea496f34b3161d@syzkaller.appspotmail.com,
syzkaller-bugs@googlegroups.com, eadavis@qq.com,
linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
huyue2@coolpad.com, linux-erofs@lists.ozlabs.org
Subject: Re: [PATCH] erofs: fix uninit-value in z_erofs_lz4_decompress
Date: Sun, 31 Dec 2023 10:32:24 +0800 [thread overview]
Message-ID: <tencent_2F7B3A6ED02C496EFC965CE96EEDFECDB40A@qq.com> (raw)
In-Reply-To: <8f0dd1ed-8849-46ef-af2a-4baf4dc91422@linux.alibaba.com>
On Sun, 31 Dec 2023 09:14:11 +0800, Gao Xiang wrote:
> > When LZ4 decompression fails, the number of bytes read from out should be
> > inputsize plus the returned overflow value ret.
> >
> > Reported-and-tested-by: syzbot+6c746eea496f34b3161d@syzkaller.appspotmail.com
> > Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> > ---
> > fs/erofs/decompressor.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c
> > index 021be5feb1bc..8ac3f96676c4 100644
> > --- a/fs/erofs/decompressor.c
> > +++ b/fs/erofs/decompressor.c
> > @@ -250,7 +250,8 @@ static int z_erofs_lz4_decompress_mem(struct z_erofs_lz4_decompress_ctx *ctx,
> > print_hex_dump(KERN_DEBUG, "[ in]: ", DUMP_PREFIX_OFFSET,
> > 16, 1, src + inputmargin, rq->inputsize, true);
> > print_hex_dump(KERN_DEBUG, "[out]: ", DUMP_PREFIX_OFFSET,
> > - 16, 1, out, rq->outputsize, true);
> > + 16, 1, out, (ret < 0 && rq->inputsize > 0) ?
> > + (ret + rq->inputsize) : rq->outputsize, true);
>
> It's incorrect since output decompressed buffer has no relationship
> with `rq->inputsize` and `ret + rq->inputsize` is meaningless too.
In this case, the value of ret is -12.
When LZ4_decompress_generic() fails, it will return "return (int) (- ((const char *) ip) - src) -1;"
Therefore, it can be clearly stated that the decompression has been carried out
to the 11 bytes of src, so reading the value of the first 11 bytes of out is
effective. Therefore, my patch should be more accurate as follows:
- 16, 1, out, rq->outputsize, true);
+ 16, 1, out, (ret < 0 && rq->inputsize > 0) ?
+ (0 - ret) : rq->outputsize, true);
>
> Also, the issue was already fixed by avoiding debugging messages as
> https://lore.kernel.org/r/20231227151903.2900413-1-hsiangkao@linux.alibaba.com
This just deleted the output.
BR,
Edward
prev parent reply other threads:[~2023-12-31 2:46 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-27 12:31 [syzbot] [erofs?] KMSAN: uninit-value in z_erofs_lz4_decompress (2) syzbot
2023-12-27 15:19 ` [PATCH] erofs: avoid debugging output for (de)compressed data Gao Xiang
2023-12-28 1:29 ` Yue Hu
2023-12-28 3:36 ` [syzbot] [erofs?] KMSAN: uninit-value in z_erofs_lz4_decompress (2) Gao Xiang
2023-12-28 4:23 ` syzbot
2023-12-29 11:09 ` [PATCH] erofs: fix uninit-value in z_erofs_lz4_decompress Edward Adam Davis via Linux-erofs
2023-12-31 1:14 ` Gao Xiang
2023-12-31 2:32 ` Edward Adam Davis via Linux-erofs [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=tencent_2F7B3A6ED02C496EFC965CE96EEDFECDB40A@qq.com \
--to=linux-erofs@lists.ozlabs.org \
--cc=eadavis@qq.com \
--cc=hsiangkao@linux.alibaba.com \
--cc=huyue2@coolpad.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=syzbot+6c746eea496f34b3161d@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).