* [PATCH 1/2] btrfs-progs: check: Avoid reading beyond item boundary for inode_ref
@ 2017-05-03 8:42 Qu Wenruo
2017-05-03 8:42 ` [PATCH 2/2] btrfs-progs: check: Avoid reading beyond item boundary for dir_item and dir_index Qu Wenruo
2017-05-03 14:53 ` [PATCH 1/2] btrfs-progs: check: Avoid reading beyond item boundary for inode_ref David Sterba
0 siblings, 2 replies; 4+ messages in thread
From: Qu Wenruo @ 2017-05-03 8:42 UTC (permalink / raw
To: linux-btrfs, dsterba
When reading out name from inode_ref, it's possible that corrupted
name_len can lead to read beyond boundary of item or even extent buffer.
This happens when checking fuzzed image /tmp/bko-161811.raw, for both
lowmem mode and original mode.
ERROR: root 5 INODE REF[256 256] doesn't have related DIR_INDEX[256 504403158265495680] namelen 0 filename filetype 0
ERROR: root 5 INODE REF[256 256] doesn't have related DIR_ITEM[256 4294967294] namelen 0 filename filetype 0
WARNING: root 5 INODE_REF[256 256] name too long
==13022== Invalid read of size 8
==13022== at 0x4C319BE: memmove (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==13022== by 0x431518: read_extent_buffer (extent_io.c:863)
==13022== by 0x474730: check_inode_ref (cmds-check.c:4307)
==13022== by 0x475D65: check_inode_item (cmds-check.c:4890)
==13022== by 0x476200: check_fs_first_inode (cmds-check.c:5011)
==13022== by 0x476276: check_fs_root_v2 (cmds-check.c:5044)
==13022== by 0x4769FB: check_fs_roots_v2 (cmds-check.c:5242)
==13022== by 0x488B5B: cmd_check (cmds-check.c:13033)
==13022== by 0x40A8C5: main (btrfs.c:246)
==13022== Address 0x5c96780 is 0 bytes after a block of size 4,224 alloc'd
==13022== at 0x4C2CF35: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==13022== by 0x4307E0: __alloc_extent_buffer (extent_io.c:538)
==13022== by 0x430C37: alloc_extent_buffer (extent_io.c:642)
==13022== by 0x413DFE: btrfs_find_create_tree_block (disk-io.c:193)
==13022== by 0x414370: read_tree_block_fs_info (disk-io.c:340)
==13022== by 0x40B5D5: read_tree_block (disk-io.h:125)
==13022== by 0x40CFD2: read_node_slot (ctree.c:652)
==13022== by 0x40E5EB: btrfs_search_slot (ctree.c:1172)
==13022== by 0x4761A8: check_fs_first_inode (cmds-check.c:5001)
==13022== by 0x476276: check_fs_root_v2 (cmds-check.c:5044)
==13022== by 0x4769FB: check_fs_roots_v2 (cmds-check.c:5242)
==13022== by 0x488B5B: cmd_check (cmds-check.c:13033)
=
Fix it by double checking inode_ref, name_len against item boundary
before trying to read out name from extent buffer, for both original
mode and lowmem mode.
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
cmds-check.c | 44 ++++++++++++++++++++++++++++++++------------
1 file changed, 32 insertions(+), 12 deletions(-)
diff --git a/cmds-check.c b/cmds-check.c
index fb2502b0..3e952742 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -1569,13 +1569,22 @@ static int process_inode_ref(struct extent_buffer *eb,
while (cur < total) {
name_len = btrfs_inode_ref_name_len(eb, ref);
index = btrfs_inode_ref_index(eb, ref);
- if (name_len <= BTRFS_NAME_LEN) {
+
+ /* inode_ref + namelen should not cross item boundary */
+ if (cur + sizeof(*ref) + name_len > total ||
+ name_len > BTRFS_NAME_LEN) {
+ if (total < cur + sizeof(*ref))
+ break;
+
+ /* Still try to read out the remaining part */
+ len = min_t(u32, total - cur - sizeof(*ref),
+ BTRFS_NAME_LEN);
+ error = REF_ERR_NAME_TOO_LONG;
+ } else {
len = name_len;
error = 0;
- } else {
- len = BTRFS_NAME_LEN;
- error = REF_ERR_NAME_TOO_LONG;
}
+
read_extent_buffer(eb, namebuf, (unsigned long)(ref + 1), len);
add_inode_backref(inode_cache, key->objectid, key->offset,
index, namebuf, len, 0, key->type, error);
@@ -4296,12 +4305,16 @@ next:
index = btrfs_inode_ref_index(node, ref);
name_len = btrfs_inode_ref_name_len(node, ref);
- if (name_len <= BTRFS_NAME_LEN) {
- len = name_len;
- } else {
- len = BTRFS_NAME_LEN;
+ if (cur + sizeof(*ref) + name_len > total ||
+ name_len > BTRFS_NAME_LEN) {
warning("root %llu INODE_REF[%llu %llu] name too long",
root->objectid, ref_key->objectid, ref_key->offset);
+
+ if (total < cur + sizeof(*ref))
+ goto out;
+ len = min_t(u32, total - cur - sizeof(*ref), BTRFS_NAME_LEN);
+ } else {
+ len = name_len;
}
read_extent_buffer(node, namebuf, (unsigned long)(ref + 1), len);
@@ -4334,6 +4347,7 @@ next:
if (cur < total)
goto next;
+out:
return err;
}
@@ -4471,16 +4485,22 @@ static int find_inode_ref(struct btrfs_root *root, struct btrfs_key *key,
if (index != (u64)-1 && index != ref_index)
goto next_ref;
- if (ref_namelen <= BTRFS_NAME_LEN) {
- len = ref_namelen;
- } else {
- len = BTRFS_NAME_LEN;
+ if (cur + sizeof(*ref) +ref_namelen > total ||
+ ref_namelen > BTRFS_NAME_LEN) {
warning("root %llu INODE %s[%llu %llu] name too long",
root->objectid,
key->type == BTRFS_INODE_REF_KEY ?
"REF" : "EXTREF",
key->objectid, key->offset);
+
+ if (cur + sizeof(*ref) > total)
+ break;
+ len = min_t(u32, total - cur - sizeof(*ref),
+ BTRFS_NAME_LEN);
+ } else {
+ len = ref_namelen;
}
+
read_extent_buffer(node, ref_namebuf, (unsigned long)(ref + 1),
len);
--
2.12.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] btrfs-progs: check: Avoid reading beyond item boundary for dir_item and dir_index
2017-05-03 8:42 [PATCH 1/2] btrfs-progs: check: Avoid reading beyond item boundary for inode_ref Qu Wenruo
@ 2017-05-03 8:42 ` Qu Wenruo
2017-05-03 15:00 ` David Sterba
2017-05-03 14:53 ` [PATCH 1/2] btrfs-progs: check: Avoid reading beyond item boundary for inode_ref David Sterba
1 sibling, 1 reply; 4+ messages in thread
From: Qu Wenruo @ 2017-05-03 8:42 UTC (permalink / raw
To: linux-btrfs, dsterba
When reading out name from inode_ref, it's possible that corrupted
name_len can lead to read beyond boundary of item or even extent buffer.
This happens when checking fuzzed image /tmp/bko-161811.raw, for both
lowmem mode and original mode.
Below is the example from lowmem mode.
ERROR: root 5 INODE REF[256 256] doesn't have related DIR_INDEX[256 216172782113783808] namelen 255 filename bar filetype 0
ERROR: root 5 INODE REF[256 256] doesn't have related DIR_ITEM[256 1306590535] namelen 255 filename bar filetype 0
WARNING: root 5 INODE[256] mode 0 shouldn't have DIR_INDEX[256 1167283096]
WARNING: root 5 DIR_ITEM[256 1167283096] name too long
==13013== Invalid read of size 1
==13013== at 0x4C31A38: memmove (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==13013== by 0x431518: read_extent_buffer (extent_io.c:863)
==13013== by 0x4752AB: check_dir_item (cmds-check.c:4627)
==13013== by 0x475E5C: check_inode_item (cmds-check.c:4911)
==13013== by 0x476200: check_fs_first_inode (cmds-check.c:5011)
==13013== by 0x476276: check_fs_root_v2 (cmds-check.c:5044)
==13013== by 0x4769FB: check_fs_roots_v2 (cmds-check.c:5242)
==13013== by 0x488B5B: cmd_check (cmds-check.c:13033)
==13013== by 0x40A8C5: main (btrfs.c:246)
==13013== Address 0x5c95b80 is 0 bytes after a block of size 4,224 alloc'd
==13013== at 0x4C2CF35: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==13013== by 0x4307E0: __alloc_extent_buffer (extent_io.c:538)
==13013== by 0x430C37: alloc_extent_buffer (extent_io.c:642)
==13013== by 0x413DFE: btrfs_find_create_tree_block (disk-io.c:193)
==13013== by 0x414370: read_tree_block_fs_info (disk-io.c:340)
==13013== by 0x40B5D5: read_tree_block (disk-io.h:125)
==13013== by 0x40CFD2: read_node_slot (ctree.c:652)
==13013== by 0x40E5EB: btrfs_search_slot (ctree.c:1172)
==13013== by 0x4761A8: check_fs_first_inode (cmds-check.c:5001)
==13013== by 0x476276: check_fs_root_v2 (cmds-check.c:5044)
==13013== by 0x4769FB: check_fs_roots_v2 (cmds-check.c:5242)
==13013== by 0x488B5B: cmd_check (cmds-check.c:13033)
Fix it by double checking dir_item, name_len against item boundary
before trying to read out name from extent buffer, for both original
mode and lowmem mode.
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
cmds-check.c | 49 +++++++++++++++++++++++++++++++++----------------
1 file changed, 33 insertions(+), 16 deletions(-)
diff --git a/cmds-check.c b/cmds-check.c
index 3e952742..520d3bff 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -1512,13 +1512,19 @@ static int process_dir_item(struct extent_buffer *eb,
filetype = btrfs_dir_type(eb, di);
rec->found_size += name_len;
- if (name_len <= BTRFS_NAME_LEN) {
+ if (cur + sizeof(*di) + name_len > total ||
+ name_len > BTRFS_NAME_LEN) {
+ error = REF_ERR_NAME_TOO_LONG;
+
+ if (cur + sizeof(*di) > total)
+ break;
+ len = min_t(u32, total - cur - sizeof(*di),
+ BTRFS_NAME_LEN);
+ } else {
len = name_len;
error = 0;
- } else {
- len = BTRFS_NAME_LEN;
- error = REF_ERR_NAME_TOO_LONG;
}
+
read_extent_buffer(eb, namebuf, (unsigned long)(di + 1), len);
if (location.type == BTRFS_INODE_ITEM_KEY) {
@@ -4235,16 +4241,22 @@ static int find_dir_item(struct btrfs_root *root, struct btrfs_key *ref_key,
if (imode_to_type(mode) != filetype)
goto next;
- if (name_len <= BTRFS_NAME_LEN) {
- len = name_len;
- } else {
- len = BTRFS_NAME_LEN;
+ if (cur + sizeof(*di) + name_len > total ||
+ name_len > BTRFS_NAME_LEN) {
warning("root %llu %s[%llu %llu] name too long %u, trimmed",
- root->objectid,
- key->type == BTRFS_DIR_ITEM_KEY ?
- "DIR_ITEM" : "DIR_INDEX",
- key->objectid, key->offset, name_len);
+ root->objectid,
+ key->type == BTRFS_DIR_ITEM_KEY ?
+ "DIR_ITEM" : "DIR_INDEX",
+ key->objectid, key->offset, name_len);
+
+ if (cur + sizeof(*di) > total)
+ break;
+ len = min_t(u32, total - cur - sizeof(*di),
+ BTRFS_NAME_LEN);
+ } else {
+ len = name_len;
}
+
read_extent_buffer(node, namebuf, (unsigned long)(di + 1), len);
if (len != namelen || strncmp(namebuf, name, len))
goto next;
@@ -4632,15 +4644,20 @@ static int check_dir_item(struct btrfs_root *root, struct btrfs_key *key,
key->objectid, key->offset, data_len);
name_len = btrfs_dir_name_len(node, di);
- if (name_len <= BTRFS_NAME_LEN) {
- len = name_len;
- } else {
- len = BTRFS_NAME_LEN;
+ if (cur + sizeof(*di) + name_len > total ||
+ name_len > BTRFS_NAME_LEN) {
warning("root %llu %s[%llu %llu] name too long",
root->objectid,
key->type == BTRFS_DIR_ITEM_KEY ?
"DIR_ITEM" : "DIR_INDEX",
key->objectid, key->offset);
+
+ if (cur + sizeof(*di) > total)
+ break;
+ len = min_t(u32, total - cur - sizeof(*di),
+ BTRFS_NAME_LEN);
+ } else {
+ len = name_len;
}
(*size) += name_len;
--
2.12.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] btrfs-progs: check: Avoid reading beyond item boundary for inode_ref
2017-05-03 8:42 [PATCH 1/2] btrfs-progs: check: Avoid reading beyond item boundary for inode_ref Qu Wenruo
2017-05-03 8:42 ` [PATCH 2/2] btrfs-progs: check: Avoid reading beyond item boundary for dir_item and dir_index Qu Wenruo
@ 2017-05-03 14:53 ` David Sterba
1 sibling, 0 replies; 4+ messages in thread
From: David Sterba @ 2017-05-03 14:53 UTC (permalink / raw
To: Qu Wenruo; +Cc: linux-btrfs, dsterba
On Wed, May 03, 2017 at 04:42:39PM +0800, Qu Wenruo wrote:
> When reading out name from inode_ref, it's possible that corrupted
> name_len can lead to read beyond boundary of item or even extent buffer.
>
> This happens when checking fuzzed image /tmp/bko-161811.raw, for both
> lowmem mode and original mode.
>
> ERROR: root 5 INODE REF[256 256] doesn't have related DIR_INDEX[256 504403158265495680] namelen 0 filename filetype 0
> ERROR: root 5 INODE REF[256 256] doesn't have related DIR_ITEM[256 4294967294] namelen 0 filename filetype 0
> WARNING: root 5 INODE_REF[256 256] name too long
> ==13022== Invalid read of size 8
> ==13022== at 0x4C319BE: memmove (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==13022== by 0x431518: read_extent_buffer (extent_io.c:863)
> ==13022== by 0x474730: check_inode_ref (cmds-check.c:4307)
> ==13022== by 0x475D65: check_inode_item (cmds-check.c:4890)
> ==13022== by 0x476200: check_fs_first_inode (cmds-check.c:5011)
> ==13022== by 0x476276: check_fs_root_v2 (cmds-check.c:5044)
> ==13022== by 0x4769FB: check_fs_roots_v2 (cmds-check.c:5242)
> ==13022== by 0x488B5B: cmd_check (cmds-check.c:13033)
> ==13022== by 0x40A8C5: main (btrfs.c:246)
> ==13022== Address 0x5c96780 is 0 bytes after a block of size 4,224 alloc'd
> ==13022== at 0x4C2CF35: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==13022== by 0x4307E0: __alloc_extent_buffer (extent_io.c:538)
> ==13022== by 0x430C37: alloc_extent_buffer (extent_io.c:642)
> ==13022== by 0x413DFE: btrfs_find_create_tree_block (disk-io.c:193)
> ==13022== by 0x414370: read_tree_block_fs_info (disk-io.c:340)
> ==13022== by 0x40B5D5: read_tree_block (disk-io.h:125)
> ==13022== by 0x40CFD2: read_node_slot (ctree.c:652)
> ==13022== by 0x40E5EB: btrfs_search_slot (ctree.c:1172)
> ==13022== by 0x4761A8: check_fs_first_inode (cmds-check.c:5001)
> ==13022== by 0x476276: check_fs_root_v2 (cmds-check.c:5044)
> ==13022== by 0x4769FB: check_fs_roots_v2 (cmds-check.c:5242)
> ==13022== by 0x488B5B: cmd_check (cmds-check.c:13033)
> =
>
> Fix it by double checking inode_ref, name_len against item boundary
> before trying to read out name from extent buffer, for both original
> mode and lowmem mode.
>
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] btrfs-progs: check: Avoid reading beyond item boundary for dir_item and dir_index
2017-05-03 8:42 ` [PATCH 2/2] btrfs-progs: check: Avoid reading beyond item boundary for dir_item and dir_index Qu Wenruo
@ 2017-05-03 15:00 ` David Sterba
0 siblings, 0 replies; 4+ messages in thread
From: David Sterba @ 2017-05-03 15:00 UTC (permalink / raw
To: Qu Wenruo; +Cc: linux-btrfs, dsterba
On Wed, May 03, 2017 at 04:42:40PM +0800, Qu Wenruo wrote:
> When reading out name from inode_ref, it's possible that corrupted
> name_len can lead to read beyond boundary of item or even extent buffer.
>
> This happens when checking fuzzed image /tmp/bko-161811.raw, for both
> lowmem mode and original mode.
>
> Below is the example from lowmem mode.
>
> ERROR: root 5 INODE REF[256 256] doesn't have related DIR_INDEX[256 216172782113783808] namelen 255 filename bar filetype 0
> ERROR: root 5 INODE REF[256 256] doesn't have related DIR_ITEM[256 1306590535] namelen 255 filename bar filetype 0
> WARNING: root 5 INODE[256] mode 0 shouldn't have DIR_INDEX[256 1167283096]
> WARNING: root 5 DIR_ITEM[256 1167283096] name too long
> ==13013== Invalid read of size 1
> ==13013== at 0x4C31A38: memmove (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==13013== by 0x431518: read_extent_buffer (extent_io.c:863)
> ==13013== by 0x4752AB: check_dir_item (cmds-check.c:4627)
> ==13013== by 0x475E5C: check_inode_item (cmds-check.c:4911)
> ==13013== by 0x476200: check_fs_first_inode (cmds-check.c:5011)
> ==13013== by 0x476276: check_fs_root_v2 (cmds-check.c:5044)
> ==13013== by 0x4769FB: check_fs_roots_v2 (cmds-check.c:5242)
> ==13013== by 0x488B5B: cmd_check (cmds-check.c:13033)
> ==13013== by 0x40A8C5: main (btrfs.c:246)
> ==13013== Address 0x5c95b80 is 0 bytes after a block of size 4,224 alloc'd
> ==13013== at 0x4C2CF35: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==13013== by 0x4307E0: __alloc_extent_buffer (extent_io.c:538)
> ==13013== by 0x430C37: alloc_extent_buffer (extent_io.c:642)
> ==13013== by 0x413DFE: btrfs_find_create_tree_block (disk-io.c:193)
> ==13013== by 0x414370: read_tree_block_fs_info (disk-io.c:340)
> ==13013== by 0x40B5D5: read_tree_block (disk-io.h:125)
> ==13013== by 0x40CFD2: read_node_slot (ctree.c:652)
> ==13013== by 0x40E5EB: btrfs_search_slot (ctree.c:1172)
> ==13013== by 0x4761A8: check_fs_first_inode (cmds-check.c:5001)
> ==13013== by 0x476276: check_fs_root_v2 (cmds-check.c:5044)
> ==13013== by 0x4769FB: check_fs_roots_v2 (cmds-check.c:5242)
> ==13013== by 0x488B5B: cmd_check (cmds-check.c:13033)
>
> Fix it by double checking dir_item, name_len against item boundary
> before trying to read out name from extent buffer, for both original
> mode and lowmem mode.
>
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-05-03 15:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-03 8:42 [PATCH 1/2] btrfs-progs: check: Avoid reading beyond item boundary for inode_ref Qu Wenruo
2017-05-03 8:42 ` [PATCH 2/2] btrfs-progs: check: Avoid reading beyond item boundary for dir_item and dir_index Qu Wenruo
2017-05-03 15:00 ` David Sterba
2017-05-03 14:53 ` [PATCH 1/2] btrfs-progs: check: Avoid reading beyond item boundary for inode_ref David Sterba
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.