[bug report] btrfs: do an allocation earlier during snapshot creation

[bug report] btrfs: do an allocation earlier during snapshot creation

[Dan Carpenter]

[ Sorry David, the complain script uses git blame and it just picked
  you.  And then I decided to keep the address because your email is
  still active and you seem like an expert.  -dan ]

Hello David Sterba,

The patch a1ee73626844: "btrfs: do an allocation earlier during
snapshot creation" from Nov 10, 2015, leads to the following static
checker warning:

	fs/btrfs/ioctl.c:890 create_snapshot()
	warn: 'pending_snapshot' not removed from list

fs/btrfs/ioctl.c
   809  
   810          pending_snapshot = kzalloc(sizeof(*pending_snapshot), GFP_KERNEL);
                ^^^^^^^^^^^^^^^^
Allocated here.

   811          if (!pending_snapshot)
   812                  return -ENOMEM;
   813  
   814          ret = get_anon_bdev(&pending_snapshot->anon_dev);
   815          if (ret < 0)
   816                  goto free_pending;
   817          pending_snapshot->root_item = kzalloc(sizeof(struct btrfs_root_item),
   818                          GFP_KERNEL);
   819          pending_snapshot->path = btrfs_alloc_path();
   820          if (!pending_snapshot->root_item || !pending_snapshot->path) {
   821                  ret = -ENOMEM;
   822                  goto free_pending;
   823          }
   824  
   825          btrfs_init_block_rsv(&pending_snapshot->block_rsv,
   826                               BTRFS_BLOCK_RSV_TEMP);
   827          /*
   828           * 1 - parent dir inode
   829           * 2 - dir entries
   830           * 1 - root item
   831           * 2 - root ref/backref
   832           * 1 - root of snapshot
   833           * 1 - UUID item
   834           */
   835          ret = btrfs_subvolume_reserve_metadata(BTRFS_I(dir)->root,
   836                                          &pending_snapshot->block_rsv, 8,
   837                                          false);
   838          if (ret)
   839                  goto free_pending;
   840  
   841          pending_snapshot->dentry = dentry;
   842          pending_snapshot->root = root;
   843          pending_snapshot->readonly = readonly;
   844          pending_snapshot->dir = dir;
   845          pending_snapshot->inherit = inherit;
   846  
   847          trans = btrfs_start_transaction(root, 0);
   848          if (IS_ERR(trans)) {
   849                  ret = PTR_ERR(trans);
   850                  goto fail;
   851          }
   852  
   853          spin_lock(&fs_info->trans_lock);
   854          list_add(&pending_snapshot->list,
   855                   &trans->transaction->pending_snapshots);
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Added to the list here.

   856          spin_unlock(&fs_info->trans_lock);
   857  
   858          ret = btrfs_commit_transaction(trans);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This will clean empty the ->pending_snapshots list when we call
create_pending_snapshots() but it's possible to fail before that
happens.

Possibly one fix would be to clear out the ->pending_snapshots list in
the __btrfs_end_transaction() function.

   859          if (ret)
   860                  goto fail;
   861  
   862          ret = pending_snapshot->error;
   863          if (ret)
   864                  goto fail;
   865  
   866          ret = btrfs_orphan_cleanup(pending_snapshot->snap);
   867          if (ret)
   868                  goto fail;
   869  
   870          inode = btrfs_lookup_dentry(d_inode(dentry->d_parent), dentry);
   871          if (IS_ERR(inode)) {
   872                  ret = PTR_ERR(inode);
   873                  goto fail;
   874          }
   875  
   876          d_instantiate(dentry, inode);
   877          ret = 0;
   878          pending_snapshot->anon_dev = 0;
   879  fail:
   880          /* Prevent double freeing of anon_dev */
   881          if (ret && pending_snapshot->snap)
   882                  pending_snapshot->snap->anon_dev = 0;
   883          btrfs_put_root(pending_snapshot->snap);
   884          btrfs_subvolume_release_metadata(root, &pending_snapshot->block_rsv);
   885  free_pending:
   886          if (pending_snapshot->anon_dev)
   887                  free_anon_bdev(pending_snapshot->anon_dev);
   888          kfree(pending_snapshot->root_item);
   889          btrfs_free_path(pending_snapshot->path);
   890          kfree(pending_snapshot);
                ^^^^^^^^^^^^^^^^^^^^^^^
If btrfs_commit_transaction() fails too early then this is freed but
it's still in the ->pending_snapshots list leading to a use after free.

   891  
   892          return ret;
   893  }

regards,
dan carpenter