| Message ID | 1511520092-37101-1-git-send-email-gujx@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Nov 24, 2017 at 06:41:28PM +0800, Gu Jinxiang wrote: > The following test failed becasuse share_data_ref be added into > extent_cache when deal with root tree node. The whole function run_next_block would need to be revisited with respect to error handling and sanity checks. Adding them only when a fuzzed image hits the particular code path is not IMHO the best approach. If there's some fuzzed test case, we should try to find all similar missing checks and fix them before moving to another type. Addressing only the failed tests gives a false sense of fixing, there are usally more similar bugs. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2017年11月29日 03:02, David Sterba wrote: > On Fri, Nov 24, 2017 at 06:41:28PM +0800, Gu Jinxiang wrote: >> The following test failed becasuse share_data_ref be added into >> extent_cache when deal with root tree node. > > The whole function run_next_block would need to be revisited with > respect to error handling and sanity checks. Adding them only when a > fuzzed image hits the particular code path is not IMHO the best > approach. As suggested before, backporting tree-checker from kernel may be a good solution for all later possible fuzzed problem. (And can make btrfs-progs become a good testbed for tree-checker related patches before merging into kernel) For this particular case, planned key->type check against root should handle it quite well. Thanks, Qu > > If there's some fuzzed test case, we should try to find all similar > missing checks and fix them before moving to another type. Addressing > only the failed tests gives a false sense of fixing, there are usally > more similar bugs. > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On Thu, Nov 30, 2017 at 02:55:49PM +0800, Qu Wenruo wrote: > > > On 2017年11月29日 03:02, David Sterba wrote: > > On Fri, Nov 24, 2017 at 06:41:28PM +0800, Gu Jinxiang wrote: > >> The following test failed becasuse share_data_ref be added into > >> extent_cache when deal with root tree node. > > > > The whole function run_next_block would need to be revisited with > > respect to error handling and sanity checks. Adding them only when a > > fuzzed image hits the particular code path is not IMHO the best > > approach. > > As suggested before, backporting tree-checker from kernel may be a good > solution for all later possible fuzzed problem. > (And can make btrfs-progs become a good testbed for tree-checker related > patches before merging into kernel) Agreed, that would be great. > For this particular case, planned key->type check against root should > handle it quite well. Good, so I'd rather take the tree-checker route. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/cmds-check.c b/cmds-check.c index 5c822b84..71b15de4 100644 --- a/cmds-check.c +++ b/cmds-check.c @@ -8018,6 +8018,14 @@ static int run_next_block(struct btrfs_root *root, } if (key.type == BTRFS_SHARED_DATA_REF_KEY) { struct btrfs_shared_data_ref *ref; + if (root->root_key.objectid != + BTRFS_EXTENT_TREE_OBJECTID) { + error( + "invaild location of share_data_ref [%d %d] root %d", + key.objectid, key.offset, + root->root_key.objectid); + continue; + } ref = btrfs_item_ptr(buf, i, struct btrfs_shared_data_ref); add_data_backref(extent_cache,
The following test failed becasuse share_data_ref be added into extent_cache when deal with root tree node. $sudo TEST=003\* make test-fuzz cmds-check.c:5660: check_owner_ref: BUG_ON `rec->is_root` triggered, value 1 /home/adam/btrfs/btrfs-progs/btrfs[0x46a43b] /home/adam/btrfs/btrfs-progs/btrfs[0x46a529] /home/adam/btrfs/btrfs-progs/btrfs[0x479e55] /home/adam/btrfs/btrfs-progs/btrfs[0x47af81] /home/adam/btrfs/btrfs-progs/btrfs[0x47f69e] /home/adam/btrfs/btrfs-progs/btrfs[0x484680] /home/adam/btrfs/btrfs-progs/btrfs[0x484cb9] /home/adam/btrfs/btrfs-progs/btrfs[0x4884f9] /home/adam/btrfs/btrfs-progs/btrfs(cmd_check+0xb53)[0x48b7c3] /home/adam/btrfs/btrfs-progs/btrfs(main+0x127)[0x40b39d] /lib64/libc.so.6(__libc_start_main+0xea)[0x7f9ac76db03a] /home/adam/btrfs/btrfs-progs/btrfs(_start+0x2a)[0x40ac9a] failed (ignored, ret=134): /home/adam/btrfs/btrfs-progs/btrfs check --check-data-csum /home/adam/btrfs/btrfs-progs/tests/fuzz-tests/images/bko-156731.raw.restored mayfail: returned code 134 (SIGABRT), not ignored test failed for case 003-multi-check-unmounted Signed-off-by: Gu Jinxiang <gujx@cn.fujitsu.com> --- cmds-check.c | 8 ++++++++ 1 file changed, 8 insertions(+)