| Message ID | 8e502fc69ea68d1647707d947e0e4625f0dd1af0.1733934886.git.fdmanana@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | btrfs: fix use-after-free when COWing tree bock and tracing is enabled | expand |
在 2024/12/12 03:11, fdmanana@kernel.org 写道: > From: Filipe Manana <fdmanana@suse.com> > > When a COWing a tree block, at btrfs_cow_block(), and we have the > tracepoint trace_btrfs_cow_block() enabled and preemption is also enabled > (CONFIG_PREEMPT=y), we can trigger a use-after-free in the COWed extent > buffer while inside the tracepoint code. This is because in some paths > that call btrfs_cow_block(), such as btrfs_search_slot(), we are holding > the last reference on the extent buffer @buf so btrfs_force_cow_block() > drops the last reference on the @buf extent buffer when it calls > free_extent_buffer_stale(buf), which schedules the release of the extent > buffer with RCU. This means that if we are on a kernel with preemption, > the current task may be preempted before calling trace_btrfs_cow_block() > and the extent buffer already released by the time trace_btrfs_cow_block() > is called, resulting in a use-after-free. > > Fix this by grabbing an extra reference on the extent buffer before > calling btrfs_force_cow_block() at btrfs_cow_block(), and then dropping > it after calling the tracepoint. > > Reported-by: syzbot+8517da8635307182c8a5@syzkaller.appspotmail.com > Link: https://lore.kernel.org/linux-btrfs/6759a9b9.050a0220.1ac542.000d.GAE@google.com/ > Signed-off-by: Filipe Manana <fdmanana@suse.com> Thanks for pinning down the error. Although considering there is really only one caller of trace_btrfs_cow_block(), can we just move this only caller into btrfs_force_cow_block() before freeing @buf? Thanks, Qu > --- > fs/btrfs/ctree.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c > index 693dc27ffb89..3a28e77b6d72 100644 > --- a/fs/btrfs/ctree.c > +++ b/fs/btrfs/ctree.c > @@ -751,10 +751,21 @@ int btrfs_cow_block(struct btrfs_trans_handle *trans, > * Also We don't care about the error, as it's handled internally. > */ > btrfs_qgroup_trace_subtree_after_cow(trans, root, buf); > + /* > + * When we are called from btrfs_search_slot() for example, we are > + * not holding an extra reference on @buf so btrfs_force_cow_block() > + * does a free_extent_buffer_stale() on the last reference and schedules > + * the extent buffer release with RCU, so we can trigger a > + * use-after-free in the trace_btrfs_cow_block() call below in case > + * preemption is enabled (CONFIG_PREEMPT=y). So grab an extra reference > + * to prevent that. > + */ > + atomic_inc(&buf->refs); > ret = btrfs_force_cow_block(trans, root, buf, parent, parent_slot, > cow_ret, search_start, 0, nest); > > trace_btrfs_cow_block(root, buf, *cow_ret); > + free_extent_buffer_stale(buf); > > return ret; > }
On Wed, Dec 11, 2024 at 8:18 PM Qu Wenruo <wqu@suse.com> wrote: > > > > 在 2024/12/12 03:11, fdmanana@kernel.org 写道: > > From: Filipe Manana <fdmanana@suse.com> > > > > When a COWing a tree block, at btrfs_cow_block(), and we have the > > tracepoint trace_btrfs_cow_block() enabled and preemption is also enabled > > (CONFIG_PREEMPT=y), we can trigger a use-after-free in the COWed extent > > buffer while inside the tracepoint code. This is because in some paths > > that call btrfs_cow_block(), such as btrfs_search_slot(), we are holding > > the last reference on the extent buffer @buf so btrfs_force_cow_block() > > drops the last reference on the @buf extent buffer when it calls > > free_extent_buffer_stale(buf), which schedules the release of the extent > > buffer with RCU. This means that if we are on a kernel with preemption, > > the current task may be preempted before calling trace_btrfs_cow_block() > > and the extent buffer already released by the time trace_btrfs_cow_block() > > is called, resulting in a use-after-free. > > > > Fix this by grabbing an extra reference on the extent buffer before > > calling btrfs_force_cow_block() at btrfs_cow_block(), and then dropping > > it after calling the tracepoint. > > > > Reported-by: syzbot+8517da8635307182c8a5@syzkaller.appspotmail.com > > Link: https://lore.kernel.org/linux-btrfs/6759a9b9.050a0220.1ac542.000d.GAE@google.com/ > > Signed-off-by: Filipe Manana <fdmanana@suse.com> > > Thanks for pinning down the error. > > Although considering there is really only one caller of > trace_btrfs_cow_block(), can we just move this only caller into > btrfs_force_cow_block() before freeing @buf? That's actually simpler and more efficient, yes. Done in v2. Thanks. > > Thanks, > Qu > > > --- > > fs/btrfs/ctree.c | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c > > index 693dc27ffb89..3a28e77b6d72 100644 > > --- a/fs/btrfs/ctree.c > > +++ b/fs/btrfs/ctree.c > > @@ -751,10 +751,21 @@ int btrfs_cow_block(struct btrfs_trans_handle *trans, > > * Also We don't care about the error, as it's handled internally. > > */ > > btrfs_qgroup_trace_subtree_after_cow(trans, root, buf); > > + /* > > + * When we are called from btrfs_search_slot() for example, we are > > + * not holding an extra reference on @buf so btrfs_force_cow_block() > > + * does a free_extent_buffer_stale() on the last reference and schedules > > + * the extent buffer release with RCU, so we can trigger a > > + * use-after-free in the trace_btrfs_cow_block() call below in case > > + * preemption is enabled (CONFIG_PREEMPT=y). So grab an extra reference > > + * to prevent that. > > + */ > > + atomic_inc(&buf->refs); > > ret = btrfs_force_cow_block(trans, root, buf, parent, parent_slot, > > cow_ret, search_start, 0, nest); > > > > trace_btrfs_cow_block(root, buf, *cow_ret); > > + free_extent_buffer_stale(buf); > > > > return ret; > > } >
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c index 693dc27ffb89..3a28e77b6d72 100644 --- a/fs/btrfs/ctree.c +++ b/fs/btrfs/ctree.c @@ -751,10 +751,21 @@ int btrfs_cow_block(struct btrfs_trans_handle *trans, * Also We don't care about the error, as it's handled internally. */ btrfs_qgroup_trace_subtree_after_cow(trans, root, buf); + /* + * When we are called from btrfs_search_slot() for example, we are + * not holding an extra reference on @buf so btrfs_force_cow_block() + * does a free_extent_buffer_stale() on the last reference and schedules + * the extent buffer release with RCU, so we can trigger a + * use-after-free in the trace_btrfs_cow_block() call below in case + * preemption is enabled (CONFIG_PREEMPT=y). So grab an extra reference + * to prevent that. + */ + atomic_inc(&buf->refs); ret = btrfs_force_cow_block(trans, root, buf, parent, parent_slot, cow_ret, search_start, 0, nest); trace_btrfs_cow_block(root, buf, *cow_ret); + free_extent_buffer_stale(buf); return ret; }