On Wed, Jul 1, 2020 at 9:23 PM Josef Bacik <josef@xxxxxxxxxxxxxx> wrote:
>
> While debugging a patch that I wrote I was hitting UAF panics when
> accessing block groups on unmount. This turned out to be because in the
> nocow case if we bail out of doing the nocow for whatever reason we need
> to call btrfs_dec_nocow_writers() if we called the inc. This puts our
> block group, but a few error cases does
>
> if (nocow) {
> btrfs_dec_nocow_writers();
> goto error;
> }
>
> unfortunately, error is
>
> error:
> if (nocow)
> btrfs_dec_nocow_writers();
>
> so we get a double put on our block group. Fix this by dropping the
> error cases calling of btrfs_dec_nocow_writers(), as it's handled at the
> error label now.
>
> Fixes: 762bf09893b4 ("btrfs: improve error handling in run_delalloc_nocow")
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
Reviewed-by: Filipe Manana <fdmanana@xxxxxxxx>
Looks good and the patch compiles and works just fine here on the
latest misc-next.
Thanks.
> ---
> fs/btrfs/inode.c | 9 +--------
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index d301550b9c70..cb18b1a13dca 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -1694,12 +1694,8 @@ static noinline int run_delalloc_nocow(struct inode *inode,
> ret = fallback_to_cow(inode, locked_page, cow_start,
> found_key.offset - 1,
> page_started, nr_written);
> - if (ret) {
> - if (nocow)
> - btrfs_dec_nocow_writers(fs_info,
> - disk_bytenr);
> + if (ret)
> goto error;
> - }
> cow_start = (u64)-1;
> }
>
> @@ -1715,9 +1711,6 @@ static noinline int run_delalloc_nocow(struct inode *inode,
> ram_bytes, BTRFS_COMPRESS_NONE,
> BTRFS_ORDERED_PREALLOC);
> if (IS_ERR(em)) {
> - if (nocow)
> - btrfs_dec_nocow_writers(fs_info,
> - disk_bytenr);
> ret = PTR_ERR(em);
> goto error;
> }
> --
> 2.24.1
>
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”
