linux-kernel - Re: [PATCH] btrfs: resume qgroup rescan on rw remount

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 10 Jul 2017 16:56:36 +0300
From:   Nikolay Borisov <nborisov@...e.com>
To:     Aleksa Sarai <asarai@...e.de>, Chris Mason <clm@...com>,
        Josef Bacik <jbacik@...com>, David Sterba <dsterba@...e.com>
Cc:     linux-btrfs@...r.kernel.org, linux-kernel@...r.kernel.org,
        stable@...r.kernel.org, Jeff Mahoney <jeffm@...e.com>
Subject: Re: [PATCH] btrfs: resume qgroup rescan on rw remount



On 10.07.2017 16:12, Nikolay Borisov wrote:
> 
> 
> On  4.07.2017 14:49, Aleksa Sarai wrote:
>> Several distributions mount the "proper root" as ro during initrd and
>> then remount it as rw before pivot_root(2). Thus, if a rescan had been
>> aborted by a previous shutdown, the rescan would never be resumed.
>>
>> This issue would manifest itself as several btrfs ioctl(2)s causing the
>> entire machine to hang when btrfs_qgroup_wait_for_completion was hit
>> (due to the fs_info->qgroup_rescan_running flag being set but the rescan
>> itself not being resumed). Notably, Docker's btrfs storage driver makes
>> regular use of BTRFS_QUOTA_CTL_DISABLE and BTRFS_IOC_QUOTA_RESCAN_WAIT
>> (causing this problem to be manifested on boot for some machines).
>>
>> Cc: <stable@...r.kernel.org> # v3.11+
>> Cc: Jeff Mahoney <jeffm@...e.com>
>> Fixes: b382a324b60f ("Btrfs: fix qgroup rescan resume on mount")
>> Signed-off-by: Aleksa Sarai <asarai@...e.de>
> 
> Indeed, looking at the code it seems that b382a324b60f ("Btrfs: fix
> qgroup rescan resume on mount") missed adding the qgroup_rescan_resume
> in the remount path. One thing which I couldn't verify though is whether
> reading fs_info->qgroup_flags without any locking is safe from remount
> context.
> 
> During remount I don't see any locks taken that prevent operations which
> can modify qgroup_flags.
> 
> 

Further inspection reveals that the access rules to qgroup_flags are
somewhat broken so this patch doesn't really make things any worse than
they are. As such:

Reviewed-by: Nikolay Borisov <nborisov@...e.com>
Tested-by: Nikolay Borisov <nborisov@...e.com>

> 
>> ---
>>  fs/btrfs/super.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
>> index 6346876c97ea..ff6690389343 100644
>> --- a/fs/btrfs/super.c
>> +++ b/fs/btrfs/super.c
>> @@ -1821,6 +1821,8 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data)
>>  			goto restore;
>>  		}
>>  
>> +		btrfs_qgroup_rescan_resume(fs_info);
>> +
>>  		if (!fs_info->uuid_root) {
>>  			btrfs_info(fs_info, "creating UUID tree");
>>  			ret = btrfs_create_uuid_tree(fs_info);
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>