linux-kernel - Re: [PATCH] btrfs: update bytes_may_use in btrfs_free_reserved

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAKrof1NNK=BAUoTb0cP+8N+68NsSRfHuJ7k47UVv_h-hW_GKGA@mail.gmail.com>
Date:   Mon, 31 Oct 2022 23:44:28 +0800
From:   Hawkins Jiawei <yin31149@...il.com>
To:     Josef Bacik <josef@...icpanda.com>
Cc:     Chris Mason <clm@...com>, David Sterba <dsterba@...e.com>,
        18801353760@....com, linux-btrfs@...r.kernel.org,
        linux-kernel@...r.kernel.org, llvm@...ts.linux.dev,
        nathan@...nel.org, ndesaulniers@...gle.com,
        syzbot+dde7e853812ed57835ea@...kaller.appspotmail.com,
        syzkaller-bugs@...glegroups.com, trix@...hat.com
Subject: Re: [PATCH] btrfs: update bytes_may_use in btrfs_free_reserved_bytes()

Hi Josef,

On Mon, 31 Oct 2022 at 23:12, Josef Bacik <josef@...icpanda.com> wrote:
>
> On Mon, Oct 31, 2022 at 12:22:24AM +0800, Hawkins Jiawei wrote:
> > Syzkaller reports warning as follows:
> > =====================================
> > WARNING: CPU: 0 PID: 3612 at fs/btrfs/space-info.h:122
> >   btrfs_space_info_free_bytes_may_use fs/btrfs/space-info.h:154 [inline]
> > WARNING: CPU: 0 PID: 3612 at fs/btrfs/space-info.h:122
> >   block_rsv_release_bytes fs/btrfs/block-rsv.c:151 [inline]
> > WARNING: CPU: 0 PID: 3612 at fs/btrfs/space-info.h:122
> >   btrfs_block_rsv_release+0x5d1/0x730 fs/btrfs/block-rsv.c:295
> > Modules linked in:
> > [...]
> > RIP: 0010:btrfs_space_info_update_bytes_may_use
> >   fs/btrfs/space-info.h:122 [inline]
> > RIP: 0010:btrfs_space_info_free_bytes_may_use
> >   fs/btrfs/space-info.h:154 [inline]
> > RIP: 0010:block_rsv_release_bytes
> >   fs/btrfs/block-rsv.c:151 [inline]
> > RIP: 0010:btrfs_block_rsv_release+0x5d1/0x730
> >   fs/btrfs/block-rsv.c:295
> > [...]
> > Call Trace:
> >  <TASK>
> >  btrfs_release_global_block_rsv+0x2f/0x250 fs/btrfs/block-rsv.c:463
> >  btrfs_free_block_groups+0xb67/0xfd0 fs/btrfs/block-group.c:4053
> >  close_ctree+0x6c5/0xbde fs/btrfs/disk-io.c:4710
> >  generic_shutdown_super+0x130/0x310 fs/super.c:491
> >  kill_anon_super+0x36/0x60 fs/super.c:1085
> >  btrfs_kill_super+0x3d/0x50 fs/btrfs/super.c:2441
> >  deactivate_locked_super+0xa7/0xf0 fs/super.c:331
> >  cleanup_mnt+0x4ce/0x560 fs/namespace.c:1186
> >  task_work_run+0x146/0x1c0 kernel/task_work.c:177
> >  ptrace_notify+0x29a/0x340 kernel/signal.c:2354
> >  ptrace_report_syscall include/linux/ptrace.h:420 [inline]
> >  ptrace_report_syscall_exit include/linux/ptrace.h:482 [inline]
> >  syscall_exit_work+0x8c/0xe0 kernel/entry/common.c:249
> >  syscall_exit_to_user_mode_prepare+0x63/0xc0 kernel/entry/common.c:276
> >  __syscall_exit_to_user_mode_work kernel/entry/common.c:281 [inline]
> >  syscall_exit_to_user_mode+0xa/0x60 kernel/entry/common.c:294
> >  do_syscall_64+0x49/0xb0 arch/x86/entry/common.c:86
> >  entry_SYSCALL_64_after_hwframe+0x63/0xcd
> >  [...]
> >  </TASK>
> > =====================================
> >
> > In btrfs_new_extent_direct(), kernel will reserves space for extent
> > by btrfs_reserve_extent(), and frees those space by
> > btrfs_free_reserved_extent() if btrfs_create_dio_extent() fails.
> >
> > Yet the problem is that, it may not update the space
> > info correctly. To be more specific, kernel will
> > converts space from ->bytes_may_use to ->bytes_reserved, in
> > btrfs_add_reserved_bytes() when reserving space.
> > But when freeing those space in btrfs_free_reserved_bytes(),
> > kernel does not convert space from ->bytes_reserved back to
> > ->bytes_may_use, which triggers the above warning.
> >
> > This patch solves it by converting space from ->bytes_reserved
> > back to ->bytes_may_use in btrfs_free_reserved_bytes().
> >
>
> This isn't correct.  I haven't looked at the code yet, but reservations go into
> ->bytes_may_use, and then when we reserve the space we subtract the reservation
> from ->bytes_may_use and add it to ->bytes_reserved.  If we free the reserved
> extent we only have to update ->bytes_reserved.  What may be happening here is
> we're failing to free the rest of our ->bytes_may_use resrvation, and that part
> needs to be addressed.  This fix as it stands however is incorrect.  Thanks,
Thanks for your explanation! I will re-analyse this bug in the way you
suggested.

>
> Josef