On Mon, Aug 06, 2018 at 06:12:37PM +0800, Anand Jain wrote: > test case btrfs/164 reported UAF.. > > [ 6712.084324] general protection fault: 0000 [#1] PREEMPT SMP > :: > [ 6712.195423] btrfs_update_commit_device_size+0x75/0xf0 [btrfs] > [ 6712.201424] btrfs_commit_transaction+0x57d/0xa90 [btrfs] > [ 6712.206999] btrfs_rm_device+0x627/0x850 [btrfs] > [ 6712.211800] btrfs_ioctl+0x2b03/0x3120 [btrfs] > :: > > reason for this is that btrfs_shrink_device() adds the device resized to > the fs_devices::resized_devices after it has called the last commit > transaction. > So the list fs_devices::resized_devices is not empty when > btrfs_shrink_device() returns. > Now the parent function btrfs_rm_device() calls > btrfs_close_bdev(device); > call_rcu(&device->rcu, free_device_rcu); > and then does the commit transaction. > The commit transaction goes through the fs_devices::resized_devices > in btrfs_update_commit_device_size() and leads to UAF. > > Fix this by making sure btrfs_shrink_device() calls the last needed > btrfs_commit_transaction() before the return. > > Reported-by: Lu Fengqi <lufq.fnst@xxxxxxxxxxxxxx> > Signed-off-by: Anand Jain <anand.jain@xxxxxxxxxx> > Tested-by: Lu Fengqi <lufq.fnst@xxxxxxxxxxxxxx> The transaction commit makes more sense here, also is consistent with what grow does. Reviewed-by: David Sterba <dsterba@xxxxxxxx>
