On 25.04.2018 18:18, Anand Jain wrote: > > > On 04/25/2018 09:16 PM, David Sterba wrote: >> On Wed, Apr 25, 2018 at 03:53:29PM +0300, Nikolay Borisov wrote: >>> Commit ddd93ef3b9d6 ("btrfs: track running balance in a simpler way") >>> which introduced this bit assigned it number 17. Unfortunately this bit >>> is already occupied by the BTRFS_FS_NEED_ASYNC_COMMIT bit. > > >>> This was >>> causing bit 17 to be cleared while __btrfs_balance was running which >>> resulted in fs_info->balance_ctl being deleted > > Any idea which thread deleted the fs_info::balance_ctl while balance > was still running?
So what happened is that the test's btrfs balance stop ioctl was allowed to proceed while __btrfs_balance was running. And this was due to bit 17 being cleared by btrfs_commit_transaction. > > Thanks for catching. > -Anand > > >>> while we have balance >>> running. This manifested in an UAF crash. Fix it by putting the >>> definition of the newly introduced balance bit after NEED_ASYNC_COMMIT >>> and giving it number 18. >>> >>> Fixes: ddd93ef3b9d6 ("btrfs: track running balance in a simpler way") >> >> Uh, thanks for catching it. The bit was free when the volume mutex >> removal patchset was in development, but the number 17 got used by the >> recent qgroup patch and I did not adjust it afterwards. >> >>> Signed-off-by: Nikolay Borisov <nbori...@suse.com> >>> --- >>> fs/btrfs/ctree.h | 12 ++++++------ >>> 1 file changed, 6 insertions(+), 6 deletions(-) >>> >>> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h >>> index 59998d5f6985..5a7893d7c668 100644 >>> --- a/fs/btrfs/ctree.h >>> +++ b/fs/btrfs/ctree.h >>> @@ -733,16 +733,16 @@ struct btrfs_delayed_root; >>> */ >>> #define BTRFS_FS_EXCL_OP 16 >>> /* >>> - * Indicate that balance has been set up from the ioctl and is in >>> the main >>> - * phase. The fs_info::balance_ctl is initialized. >>> - */ >>> -#define BTRFS_FS_BALANCE_RUNNING 17 >>> - >>> -/* >>> * To info transaction_kthread we need an immediate commit so it >>> doesn't >>> * need to wait for commit_interval >>> */ >>> #define BTRFS_FS_NEED_ASYNC_COMMIT 17 >>> +/* >>> + * Indicate that balance has been set up from the ioctl and is in >>> the main >>> + * phase. The fs_info::balance_ctl is initialized. >>> + */ >>> +#define BTRFS_FS_BALANCE_RUNNING 18 >> >> I'll fold the fix so we don't have an intermediate breakage in the >> history. >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in >> the body of a message to majord...@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html