Messages in this thread | | | From | Richard Weinberger <> | Date | Sat, 31 Oct 2020 22:13:16 +0100 | Subject | Re: [PATCH RFC 0/5] ubifs: Prevent memory oob accessing while dumping node |
| |
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
| |