On 2019/3/18 下午11:45, Nikolay Borisov wrote:
> If 'cur_level' is 7 then the bound checking at the top of the function
> will actually pass. Later on, it's possible to dereference
> ds_path->nodes[cur_level+1] which will be an out of bounds.
>
> The correct check will be cur_level >= BTRFS_MAX_LEVEL - 1 .
>
> Fixes-coverty-id: 1440918
> Fixes-coverty-id: 1440911
> Fixes: ea49f3e73c4b ("btrfs: qgroup: Introduce function to find all new tree blocks of reloc tree")
> Signed-off-by: Nikolay Borisov <nborisov@xxxxxxxx>
Reviewed-by: Qu Wenruo <wqu@xxxxxxxx>
Thanks,
Qu
> ---
> fs/btrfs/qgroup.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index eb680b715dd6..7019edf5625c 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -1922,8 +1922,8 @@ static int qgroup_trace_new_subtree_blocks(struct btrfs_trans_handle* trans,
> int i;
>
> /* Level sanity check */
> - if (cur_level < 0 || cur_level >= BTRFS_MAX_LEVEL ||
> - root_level < 0 || root_level >= BTRFS_MAX_LEVEL ||
> + if (cur_level < 0 || cur_level >= BTRFS_MAX_LEVEL - 1 ||
> + root_level < 0 || root_level >= BTRFS_MAX_LEVEL - 1 ||
> root_level < cur_level) {
> btrfs_err_rl(fs_info,
> "%s: bad levels, cur_level=%d root_level=%d",
> @@ -3482,7 +3482,7 @@ static int __btrfs_qgroup_release_data(struct inode *inode,
> if (free && reserved)
> return qgroup_free_reserved_data(inode, reserved, start, len);
> extent_changeset_init(&changeset);
> - ret = clear_record_extent_bits(&BTRFS_I(inode)->io_tree, start,
> + ret = clear_record_extent_bits(&BTRFS_I(inode)->io_tree, start,
> start + len -1, EXTENT_QGROUP_RESERVED, &changeset);
> if (ret < 0)
> goto out;
>
