| Message ID | 20200727142805.4896-1-josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] btrfs: only search for left_info if there is no right_info | expand |
On Mon, Jul 27, 2020 at 10:28:05AM -0400, Josef Bacik wrote: > In try_to_merge_free_space we attempt to find entries to the left and > right of the entry we are adding to see if they can be merged. We > search for an entry past our current info (saved into right_info), and > then if right_info exists and it has a rb_prev() we save the rb_prev() > into left_info. > > However there's a slight problem in the case that we have a right_info, > but no entry previous to that entry. At that point we will search for > an entry just before the info we're attempting to insert. This will > simply find right_info again, and assign it to left_info, making them > both the same pointer. > > Now if right_info _can_ be merged with the range we're inserting, we'll > add it to the info and free right_info. However further down we'll > access left_info, which was right_info, and thus get a UAF. > > Fix this by only searching for the left entry if we don't find a right > entry at all. > > The CVE referenced had a specially crafted file system that could > trigger this UAF. However with the tree checker improvements we no > longer trigger the conditions for the UAF. But the original conditions > still apply, hence this fix. > > Reference: CVE-2019-19448 > Fixes: 963030817060 ("Btrfs: use hybrid extents+bitmap rb tree for free space") > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > --- > v2->v3: > - Updated the changelog. Added to misc-next, thanks.
For reasons unrelated to btrfs I've been trying linux-next-20200728 today. This patch causes Kernel Oops and call trace (with try_merge_free_space on top of stack) on my system. Because of immediate system lock-up I can't provide a dmesg log and there's nothing in /var/log (probably because it immediately goes read-only), but removing this patch and rebuilding the kernel fixed my issues. I'm happy to help if you need more info in order to reproduce. Am Di., 28. Juli 2020 um 16:47 Uhr schrieb David Sterba <dsterba@suse.cz>: > > On Mon, Jul 27, 2020 at 10:28:05AM -0400, Josef Bacik wrote: > > In try_to_merge_free_space we attempt to find entries to the left and > > right of the entry we are adding to see if they can be merged. We > > search for an entry past our current info (saved into right_info), and > > then if right_info exists and it has a rb_prev() we save the rb_prev() > > into left_info. > > > > However there's a slight problem in the case that we have a right_info, > > but no entry previous to that entry. At that point we will search for > > an entry just before the info we're attempting to insert. This will > > simply find right_info again, and assign it to left_info, making them > > both the same pointer. > > > > Now if right_info _can_ be merged with the range we're inserting, we'll > > add it to the info and free right_info. However further down we'll > > access left_info, which was right_info, and thus get a UAF. > > > > Fix this by only searching for the left entry if we don't find a right > > entry at all. > > > > The CVE referenced had a specially crafted file system that could > > trigger this UAF. However with the tree checker improvements we no > > longer trigger the conditions for the UAF. But the original conditions > > still apply, hence this fix. > > > > Reference: CVE-2019-19448 > > Fixes: 963030817060 ("Btrfs: use hybrid extents+bitmap rb tree for free space") > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > > --- > > v2->v3: > > - Updated the changelog. > > Added to misc-next, thanks.
On 7/29/20 11:42 AM, Sebastian Döring wrote: > For reasons unrelated to btrfs I've been trying linux-next-20200728 today. > > This patch causes Kernel Oops and call trace (with > try_merge_free_space on top of stack) on my system. Because of > immediate system lock-up I can't provide a dmesg log and there's > nothing in /var/log (probably because it immediately goes read-only), > but removing this patch and rebuilding the kernel fixed my issues. I'm > happy to help if you need more info in order to reproduce. > Lol I literally just hit this and sent the fixup to Dave when you posted this. My bad, somehow it didn't hit either of us until just now. Thanks, Josef
On Wed, Jul 29, 2020 at 11:43:40AM -0400, Josef Bacik wrote: > On 7/29/20 11:42 AM, Sebastian Döring wrote: > > For reasons unrelated to btrfs I've been trying linux-next-20200728 today. > > > > This patch causes Kernel Oops and call trace (with > > try_merge_free_space on top of stack) on my system. Because of > > immediate system lock-up I can't provide a dmesg log and there's > > nothing in /var/log (probably because it immediately goes read-only), > > but removing this patch and rebuilding the kernel fixed my issues. I'm > > happy to help if you need more info in order to reproduce. > > > > Lol I literally just hit this and sent the fixup to Dave when you posted this. > My bad, somehow it didn't hit either of us until just now. Thanks, Updated misc-next pushed, for-next will follow.
On 2020/7/30 上午12:13, David Sterba wrote: > On Wed, Jul 29, 2020 at 11:43:40AM -0400, Josef Bacik wrote: >> On 7/29/20 11:42 AM, Sebastian Döring wrote: >>> For reasons unrelated to btrfs I've been trying linux-next-20200728 today. >>> >>> This patch causes Kernel Oops and call trace (with >>> try_merge_free_space on top of stack) on my system. Because of >>> immediate system lock-up I can't provide a dmesg log and there's >>> nothing in /var/log (probably because it immediately goes read-only), >>> but removing this patch and rebuilding the kernel fixed my issues. I'm >>> happy to help if you need more info in order to reproduce. >>> >> >> Lol I literally just hit this and sent the fixup to Dave when you posted this. >> My bad, somehow it didn't hit either of us until just now. Thanks, > > Updated misc-next pushed, for-next will follow. > I guess it's still not working... The latest commit 2f0cb6b46a28 ("btrfs: only search for left_info if there is no right_info in try_merge_free_space"), shows it's now the updated one. But still fails at selftest: https://paste.opensuse.org/41470779 Have to revert that commit to do my test... Thanks, Qu
On 7/30/20 7:42 AM, Qu Wenruo wrote: > > > On 2020/7/30 上午12:13, David Sterba wrote: >> On Wed, Jul 29, 2020 at 11:43:40AM -0400, Josef Bacik wrote: >>> On 7/29/20 11:42 AM, Sebastian Döring wrote: >>>> For reasons unrelated to btrfs I've been trying linux-next-20200728 today. >>>> >>>> This patch causes Kernel Oops and call trace (with >>>> try_merge_free_space on top of stack) on my system. Because of >>>> immediate system lock-up I can't provide a dmesg log and there's >>>> nothing in /var/log (probably because it immediately goes read-only), >>>> but removing this patch and rebuilding the kernel fixed my issues. I'm >>>> happy to help if you need more info in order to reproduce. >>>> >>> >>> Lol I literally just hit this and sent the fixup to Dave when you posted this. >>> My bad, somehow it didn't hit either of us until just now. Thanks, >> >> Updated misc-next pushed, for-next will follow. >> > I guess it's still not working... > > The latest commit 2f0cb6b46a28 ("btrfs: only search for left_info if > there is no right_info in try_merge_free_space"), shows it's now the > updated one. > > But still fails at selftest: > https://paste.opensuse.org/41470779 > > Have to revert that commit to do my test... > I'm looking at misc-next and the commit is c5f239232fbe749042e05cae508e1c514ed5bd3c Do you have a struct btrfs_free_space *left_info = NULL; in your tree? Thanks, Josef
On 2020/7/30 下午10:02, Josef Bacik wrote: > On 7/30/20 7:42 AM, Qu Wenruo wrote: >> >> >> On 2020/7/30 上午12:13, David Sterba wrote: >>> On Wed, Jul 29, 2020 at 11:43:40AM -0400, Josef Bacik wrote: >>>> On 7/29/20 11:42 AM, Sebastian Döring wrote: >>>>> For reasons unrelated to btrfs I've been trying linux-next-20200728 >>>>> today. >>>>> >>>>> This patch causes Kernel Oops and call trace (with >>>>> try_merge_free_space on top of stack) on my system. Because of >>>>> immediate system lock-up I can't provide a dmesg log and there's >>>>> nothing in /var/log (probably because it immediately goes read-only), >>>>> but removing this patch and rebuilding the kernel fixed my issues. I'm >>>>> happy to help if you need more info in order to reproduce. >>>>> >>>> >>>> Lol I literally just hit this and sent the fixup to Dave when you >>>> posted this. >>>> My bad, somehow it didn't hit either of us until just now. Thanks, >>> >>> Updated misc-next pushed, for-next will follow. >>> >> I guess it's still not working... >> >> The latest commit 2f0cb6b46a28 ("btrfs: only search for left_info if >> there is no right_info in try_merge_free_space"), shows it's now the >> updated one. >> >> But still fails at selftest: >> https://paste.opensuse.org/41470779 >> >> Have to revert that commit to do my test... >> > > I'm looking at misc-next and the commit is > > c5f239232fbe749042e05cae508e1c514ed5bd3c > > Do you have a > > struct btrfs_free_space *left_info = NULL; > > in your tree? Thanks, > > Josef Now it's there and no longer crash. Thanks, Qu
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. */
In try_to_merge_free_space we attempt to find entries to the left and right of the entry we are adding to see if they can be merged. We search for an entry past our current info (saved into right_info), and then if right_info exists and it has a rb_prev() we save the rb_prev() into left_info. However there's a slight problem in the case that we have a right_info, but no entry previous to that entry. At that point we will search for an entry just before the info we're attempting to insert. This will simply find right_info again, and assign it to left_info, making them both the same pointer. Now if right_info _can_ be merged with the range we're inserting, we'll add it to the info and free right_info. However further down we'll access left_info, which was right_info, and thus get a UAF. Fix this by only searching for the left entry if we don't find a right entry at all. The CVE referenced had a specially crafted file system that could trigger this UAF. However with the tree checker improvements we no longer trigger the conditions for the UAF. But the original conditions still apply, hence this fix. Reference: CVE-2019-19448 Fixes: 963030817060 ("Btrfs: use hybrid extents+bitmap rb tree for free space") Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- v2->v3: - Updated the changelog. fs/btrfs/free-space-cache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)