| Message ID | 20200722184537.19896-1-josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] btrfs: only search for left_info if there is no right_info | expand |
On Wed, Jul 22, 2020 at 02:45:37PM -0400, Josef Bacik wrote: > The CVE referenced doesn't actually trigger the problem anymore because > of the tree-checker improvements, however the underlying issue can still > happen. What was the problem? > If we find a right_info, but rb_prev() is NULL, then we're the furthest > most item in the tree currently, and there will be no left_info. > However we'll still search from offset-1, which would return right_info > again which we store in left_info. If we then free right_info we'll > have free'd left_info as well, and boom, UAF. Instead fix this check so > that if we don't have a right_info we do the search for the left_info, > otherwise left_info comes from rb_prev or is simply NULL as it should > be. > > Reference: CVE-2019-19448 > Fixes: 963030817060 ("Btrfs: use hybrid extents+bitmap rb tree for free space") > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > --- > v1->v2: > - Fixed the title, I had changed the fix but forgot to change the title in v1 The title still repeats what the code does and left_info or right_info are not terms that are understood without context (unlike eg. fs_info)
diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c index 6d961e11639e..37fd2fa1ac1f 100644 --- a/fs/btrfs/free-space-cache.c +++ b/fs/btrfs/free-space-cache.c @@ -2298,7 +2298,7 @@ static bool try_merge_free_space(struct btrfs_free_space_ctl *ctl, if (right_info && rb_prev(&right_info->offset_index)) left_info = rb_entry(rb_prev(&right_info->offset_index), struct btrfs_free_space, offset_index); - else + else if (!right_info) left_info = tree_search_offset(ctl, offset - 1, 0, 0); /* See try_merge_free_space() comment. */
The CVE referenced doesn't actually trigger the problem anymore because of the tree-checker improvements, however the underlying issue can still happen. If we find a right_info, but rb_prev() is NULL, then we're the furthest most item in the tree currently, and there will be no left_info. However we'll still search from offset-1, which would return right_info again which we store in left_info. If we then free right_info we'll have free'd left_info as well, and boom, UAF. Instead fix this check so that if we don't have a right_info we do the search for the left_info, otherwise left_info comes from rb_prev or is simply NULL as it should be. Reference: CVE-2019-19448 Fixes: 963030817060 ("Btrfs: use hybrid extents+bitmap rb tree for free space") Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- v1->v2: - Fixed the title, I had changed the fix but forgot to change the title in v1 fs/btrfs/free-space-cache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)