Re: [PATCH RFC 0/5] ubifs: Prevent memory oob accessing while dumping node
On Mon, Oct 19, 2020 at 5:13 AM Zhihao Cheng 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
Re: [PATCH RFC 0/5] ubifs: Prevent memory oob accessing while dumping node
在 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.
[PATCH RFC 0/5] ubifs: Prevent memory oob accessing while dumping node
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(-)
--
2.25.4
