btrfs: fix use-after-free due to race between replace start and cancel
authorAnand Jain <anand.jain@oracle.com>
Wed, 14 Nov 2018 05:50:26 +0000 (13:50 +0800)
committerDavid Sterba <dsterba@suse.com>
Mon, 17 Dec 2018 13:51:35 +0000 (14:51 +0100)
The device replace cancel thread can race with the replace start thread
and if fs_info::scrubs_running is not yet set, btrfs_scrub_cancel() will
fail to stop the scrub thread.

The scrub thread continues with the scrub for replace which then will
try to write to the target device and which is already freed by the
cancel thread.

scrub_setup_ctx() warns as tgtdev is NULL.

  struct scrub_ctx *scrub_setup_ctx(struct btrfs_device *dev, int is_dev_replace)
  {
  ...
  if (is_dev_replace) {
  WARN_ON(!fs_info->dev_replace.tgtdev);  <===
  sctx->pages_per_wr_bio = SCRUB_PAGES_PER_WR_BIO;
  sctx->wr_tgtdev = fs_info->dev_replace.tgtdev;
  sctx->flush_all_writes = false;
  }

  [ 6724.497655] BTRFS info (device sdb): dev_replace from /dev/sdb (devid 1) to /dev/sdc started
  [ 6753.945017] BTRFS info (device sdb): dev_replace from /dev/sdb (devid 1) to /dev/sdc canceled
  [ 6852.426700] WARNING: CPU: 0 PID: 4494 at fs/btrfs/scrub.c:622 scrub_setup_ctx.isra.19+0x220/0x230 [btrfs]
  ...
  [ 6852.428928] RIP: 0010:scrub_setup_ctx.isra.19+0x220/0x230 [btrfs]
  ...
  [ 6852.432970] Call Trace:
  [ 6852.433202]  btrfs_scrub_dev+0x19b/0x5c0 [btrfs]
  [ 6852.433471]  btrfs_dev_replace_start+0x48c/0x6a0 [btrfs]
  [ 6852.433800]  btrfs_dev_replace_by_ioctl+0x3a/0x60 [btrfs]
  [ 6852.434097]  btrfs_ioctl+0x2476/0x2d20 [btrfs]
  [ 6852.434365]  ? do_sigaction+0x7d/0x1e0
  [ 6852.434623]  do_vfs_ioctl+0xa9/0x6c0
  [ 6852.434865]  ? syscall_trace_enter+0x1c8/0x310
  [ 6852.435124]  ? syscall_trace_enter+0x1c8/0x310
  [ 6852.435387]  ksys_ioctl+0x60/0x90
  [ 6852.435663]  __x64_sys_ioctl+0x16/0x20
  [ 6852.435907]  do_syscall_64+0x50/0x180
  [ 6852.436150]  entry_SYSCALL_64_after_hwframe+0x49/0xbe

Further, as the replace thread enters scrub_write_page_to_dev_replace()
without the target device it panics:

  static int scrub_add_page_to_wr_bio(struct scrub_ctx *sctx,
      struct scrub_page *spage)
  {
  ...
bio_set_dev(bio, sbio->dev->bdev); <======

  [ 6929.715145] BUG: unable to handle kernel NULL pointer dereference at 00000000000000a0
  ..
  [ 6929.717106] Workqueue: btrfs-scrub btrfs_scrub_helper [btrfs]
  [ 6929.717420] RIP: 0010:scrub_write_page_to_dev_replace+0xb4/0x260
  [btrfs]
  ..
  [ 6929.721430] Call Trace:
  [ 6929.721663]  scrub_write_block_to_dev_replace+0x3f/0x60 [btrfs]
  [ 6929.721975]  scrub_bio_end_io_worker+0x1af/0x490 [btrfs]
  [ 6929.722277]  normal_work_helper+0xf0/0x4c0 [btrfs]
  [ 6929.722552]  process_one_work+0x1f4/0x520
  [ 6929.722805]  ? process_one_work+0x16e/0x520
  [ 6929.723063]  worker_thread+0x46/0x3d0
  [ 6929.723313]  kthread+0xf8/0x130
  [ 6929.723544]  ? process_one_work+0x520/0x520
  [ 6929.723800]  ? kthread_delayed_work_timer_fn+0x80/0x80
  [ 6929.724081]  ret_from_fork+0x3a/0x50

Fix this by letting the btrfs_dev_replace_finishing() to do the job of
cleaning after the cancel, including freeing of the target device.
btrfs_dev_replace_finishing() is called when btrfs_scub_dev() returns
along with the scrub return status.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
fs/btrfs/dev-replace.c

index 33d07c426c59ee949bf2aecbb13a4c44a080a928..08092d329f666d74fea03093bbf4ed56734cb887 100644 (file)
@@ -803,39 +803,58 @@ int btrfs_dev_replace_cancel(struct btrfs_fs_info *fs_info)
        case BTRFS_IOCTL_DEV_REPLACE_STATE_CANCELED:
                result = BTRFS_IOCTL_DEV_REPLACE_RESULT_NOT_STARTED;
                btrfs_dev_replace_write_unlock(dev_replace);
        case BTRFS_IOCTL_DEV_REPLACE_STATE_CANCELED:
                result = BTRFS_IOCTL_DEV_REPLACE_RESULT_NOT_STARTED;
                btrfs_dev_replace_write_unlock(dev_replace);
-               goto leave;
+               break;
        case BTRFS_IOCTL_DEV_REPLACE_STATE_STARTED:
        case BTRFS_IOCTL_DEV_REPLACE_STATE_STARTED:
+               result = BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_ERROR;
+               tgt_device = dev_replace->tgtdev;
+               src_device = dev_replace->srcdev;
+               btrfs_dev_replace_write_unlock(dev_replace);
+               btrfs_scrub_cancel(fs_info);
+               /* btrfs_dev_replace_finishing() will handle the cleanup part */
+               btrfs_info_in_rcu(fs_info,
+                       "dev_replace from %s (devid %llu) to %s canceled",
+                       btrfs_dev_name(src_device), src_device->devid,
+                       btrfs_dev_name(tgt_device));
+               break;
        case BTRFS_IOCTL_DEV_REPLACE_STATE_SUSPENDED:
        case BTRFS_IOCTL_DEV_REPLACE_STATE_SUSPENDED:
+               /*
+                * Scrub doing the replace isn't running so we need to do the
+                * cleanup step of btrfs_dev_replace_finishing() here
+                */
                result = BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_ERROR;
                tgt_device = dev_replace->tgtdev;
                src_device = dev_replace->srcdev;
                dev_replace->tgtdev = NULL;
                dev_replace->srcdev = NULL;
                result = BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_ERROR;
                tgt_device = dev_replace->tgtdev;
                src_device = dev_replace->srcdev;
                dev_replace->tgtdev = NULL;
                dev_replace->srcdev = NULL;
-               break;
-       }
-       dev_replace->replace_state = BTRFS_IOCTL_DEV_REPLACE_STATE_CANCELED;
-       dev_replace->time_stopped = ktime_get_real_seconds();
-       dev_replace->item_needs_writeback = 1;
-       btrfs_dev_replace_write_unlock(dev_replace);
-       btrfs_scrub_cancel(fs_info);
+               dev_replace->replace_state =
+                               BTRFS_IOCTL_DEV_REPLACE_STATE_CANCELED;
+               dev_replace->time_stopped = ktime_get_real_seconds();
+               dev_replace->item_needs_writeback = 1;
 
 
-       trans = btrfs_start_transaction(root, 0);
-       if (IS_ERR(trans)) {
-               mutex_unlock(&dev_replace->lock_finishing_cancel_unmount);
-               return PTR_ERR(trans);
-       }
-       ret = btrfs_commit_transaction(trans);
-       WARN_ON(ret);
+               btrfs_dev_replace_write_unlock(dev_replace);
 
 
-       btrfs_info_in_rcu(fs_info,
-               "dev_replace from %s (devid %llu) to %s canceled",
-               btrfs_dev_name(src_device), src_device->devid,
-               btrfs_dev_name(tgt_device));
+               btrfs_scrub_cancel(fs_info);
+
+               trans = btrfs_start_transaction(root, 0);
+               if (IS_ERR(trans)) {
+                       mutex_unlock(&dev_replace->lock_finishing_cancel_unmount);
+                       return PTR_ERR(trans);
+               }
+               ret = btrfs_commit_transaction(trans);
+               WARN_ON(ret);
 
 
-       if (tgt_device)
-               btrfs_destroy_dev_replace_tgtdev(tgt_device);
+               btrfs_info_in_rcu(fs_info,
+               "suspended dev_replace from %s (devid %llu) to %s canceled",
+                       btrfs_dev_name(src_device), src_device->devid,
+                       btrfs_dev_name(tgt_device));
+
+               if (tgt_device)
+                       btrfs_destroy_dev_replace_tgtdev(tgt_device);
+               break;
+       default:
+               result = -EINVAL;
+       }
 
 
-leave:
        mutex_unlock(&dev_replace->lock_finishing_cancel_unmount);
        return result;
 }
        mutex_unlock(&dev_replace->lock_finishing_cancel_unmount);
        return result;
 }