linux-kernel - Re: [PATCH] quota: Lock s_umount in exclusive mode for Q

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] [day] [month] [year] [list]

Date:   Thu, 20 Dec 2018 19:33:17 +0100
From:   Javier Barrio <javier.barrio.mart@...il.com>
To:     Jan Kara <jack@...e.cz>
Cc:     Jan Kara <jack@...e.com>, linux-kernel@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, linux-xfs@...r.kernel.org
Subject: Re: [PATCH] quota: Lock s_umount in exclusive mode for
 Q_XQUOTA{ON,OFF} quotactls.

On 20/12/18 9:20, Jan Kara wrote:
> On Fri 14-12-18 10:56:13, Jan Kara wrote:
>> On Thu 13-12-18 01:06:29, Javier Barrio wrote:
>>> Commit 1fa5efe3622db58cb8c7b9a50665e9eb9a6c7e97 (ext4: Use generic
>>> helpers for quotaon and quotaoff) made possible to call
>>> quotactl(Q_XQUOTAON/OFF) on ext4 filesystems with sysfile quota support.
>>> This leads to calling dquot_enable/disable without s_umount held in excl.
>>> mode, because quotactl_cmd_onoff checks only for Q_QUOTAON/OFF.
>>>
>>> The following WARN_ON_ONCE triggers (in this case for dquot_enable, ext4,
>>> latest Linus' tree):
>>>
>>> [  155.036847] WARNING: CPU: 0 PID: 2343 at fs/quota/dquot.c:2469 dquot_enable+0x34/0xb9
>>
>> ...
>>
>>> This patch locks the superblock ->s_umount sem. in exclusive mode for all Q_XQUOTAON/OFF
>>> quotactls too in addition to Q_QUOTAON/OFF.
>>
>> Thanks for the patch! It looks good to me but let me run it past XFS
>> people. Looking at XFS code they definitely do not *need* s_umount in
>> exclusive mode for Q_XQUOTAON/OFF (they have their private mutex for
>> the exclusion). Shared mode they currently get is enough for them. But
>> exclusive mode is fine for them as well AFAICT and it would be easier if
>> all quota backends had the same locking rules wrt VFS locks. XFS guys, any
>> objections to switching Q_XQUOTAON/OFF handlers from having s_umount locked
>> for read to having it locked exclusive?
> 
> Since there were no objections and the patch passed quota xfstests for XFS,
> I've added it to my tree and will push it to Linus in the coming merge
> window.
> 
> 								Honza
> 

Thank you!


	Javier