From: Richard Weinberger <richard.weinberger@gmail.com>
To: Zhihao Cheng <chengzhihao1@huawei.com>
Cc: linux-mtd@lists.infradead.org,
LKML <linux-kernel@vger.kernel.org>,
Richard Weinberger <richard@nod.at>,
liu.song11@zte.com.cn, "zhangyi (F)" <yi.zhang@huawei.com>
Subject: Re: [PATCH RFC 0/5] ubifs: Prevent memory oob accessing while dumping node
Date: Sat, 31 Oct 2020 22:13:16 +0100 [thread overview]
Message-ID: <CAFLxGvyi1hVLmQ3Q8+nGZT-wgHVjwBewmg4NdEQ=iX5svJsY2Q@mail.gmail.com> (raw)
In-Reply-To: <f22055d4-47c8-d83d-f650-6fd82ce54a29@huawei.com>
On Mon, Oct 19, 2020 at 5:13 AM Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>
> 在 2020/6/16 15:11, Zhihao Cheng 写道:
> > We use function ubifs_dump_node() to dump bad node caused by some
> > reasons (Such as bit flipping caused by hardware error, writing bypass
> > ubifs or unknown bugs in ubifs). The node content can not be trusted
> > anymore, so we should prevent memory out-of-bounds accessing while
> > dumping node in following situations:
> >
> > 1. bad node_len: Dumping data according to 'ch->len' which may exceed
> > the size of memory allocated for node.
> > 2. bad node content: Some kinds of node can record additional data, eg.
> > index node and orphan node, make sure the size of additional data
> > not beyond the node length.
> > 3. node_type changes: Read data according to type A, but expected type
> > B, before that, node is allocated according to type B's size. Length
> > of type A node is greater than type B node.
> >
> > Commit acc5af3efa303d5f3 ("ubifs: Fix out-of-bounds memory access caused
> > by abnormal value of node_len") handles situation 1 for data node only,
> > it would be better if we can solve problems in above situations for all
> > kinds of nodes.
> >
> > Patch 1 adds a new parameter 'node_len'(size of memory which is allocated
> > for the node) in function ubifs_dump_node(), safe dumping length of the
> > node should be: minimum(ch->len, c->ranges[node_type].max_len, node_len).
> > Besides, c->ranges[node_type].min_len can not greater than safe dumping
> > length, which may caused by node_type changes(situation 3).
> >
> > Patch 2 reverts commit acc5af3efa303d5f ("ubifs: Fix out-of-bounds memory
> > access caused by abnormal value of node_len") to prepare for patch 3.
> >
> > Patch 3 replaces modified function ubifs_dump_node() in all node dumping
> > places except for ubifs_dump_sleb().
> >
> > Patch 4 removes unused function ubifs_dump_sleb(),
> >
> > Patch 5 allows ubifs_dump_node() to dump all branches of the index node.
> >
> > Some tests after patchset applied:
> > https://bugzilla.kernel.org/show_bug.cgi?id=208203
> >
> > Zhihao Cheng (5):
> > ubifs: Limit dumping length by size of memory which is allocated for
> > the node
> > Revert "ubifs: Fix out-of-bounds memory access caused by abnormal
> > value of node_len"
> > ubifs: Pass node length in all node dumping callers
> > ubifs: ubifs_dump_sleb: Remove unused function
> > ubifs: ubifs_dump_node: Dump all branches of the index node
> >
> > fs/ubifs/commit.c | 4 +-
> > fs/ubifs/debug.c | 111 ++++++++++++++++++++++++++------------------
> > fs/ubifs/debug.h | 5 +-
> > fs/ubifs/file.c | 2 +-
> > fs/ubifs/io.c | 37 +++++----------
> > fs/ubifs/journal.c | 3 +-
> > fs/ubifs/master.c | 4 +-
> > fs/ubifs/orphan.c | 6 ++-
> > fs/ubifs/recovery.c | 6 +--
> > fs/ubifs/replay.c | 4 +-
> > fs/ubifs/sb.c | 2 +-
> > fs/ubifs/scan.c | 4 +-
> > fs/ubifs/super.c | 2 +-
> > fs/ubifs/tnc.c | 8 ++--
> > fs/ubifs/tnc_misc.c | 4 +-
> > fs/ubifs/ubifs.h | 4 +-
> > 16 files changed, 108 insertions(+), 98 deletions(-)
> >
> ping, although it is not a serious problem for ubifs, but dumping extra
> memory by formating specified ubifs img may cause security problem.
Thanks for reminding me, yes this needs fixing.
I'll give it a try and then apply it for next.
--
Thanks,
//richard
prev parent reply other threads:[~2020-10-31 21:13 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-16 7:11 [PATCH RFC 0/5] ubifs: Prevent memory oob accessing while dumping node Zhihao Cheng
2020-06-16 7:11 ` [PATCH RFC 1/5] ubifs: Limit dumping length by size of memory which is allocated for the node Zhihao Cheng
2020-06-16 7:11 ` [PATCH RFC 2/5] Revert "ubifs: Fix out-of-bounds memory access caused by abnormal value of node_len" Zhihao Cheng
2020-06-16 7:11 ` [PATCH RFC 3/5] ubifs: Pass node length in all node dumping callers Zhihao Cheng
2020-06-16 7:11 ` [PATCH RFC 4/5] ubifs: ubifs_dump_sleb: Remove unused function Zhihao Cheng
2020-06-16 7:11 ` [PATCH RFC 5/5] ubifs: ubifs_dump_node: Dump all branches of the index node Zhihao Cheng
2020-10-19 3:05 ` [PATCH RFC 0/5] ubifs: Prevent memory oob accessing while dumping node Zhihao Cheng
2020-10-31 21:13 ` Richard Weinberger [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='CAFLxGvyi1hVLmQ3Q8+nGZT-wgHVjwBewmg4NdEQ=iX5svJsY2Q@mail.gmail.com' \
--to=richard.weinberger@gmail.com \
--cc=chengzhihao1@huawei.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=liu.song11@zte.com.cn \
--cc=richard@nod.at \
--cc=yi.zhang@huawei.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 NNTP newsgroup(s).