linux-ext4 - Re: [PATCH] ext4: compare inode's i_projid with EXT4_DEF_PROJID rather than check EXT4_INODE

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]

Message-ID: <20211211004929.GB69182@magnolia>
Date:   Fri, 10 Dec 2021 16:49:29 -0800
From:   "Darrick J. Wong" <djwong@...nel.org>
To:     Andreas Dilger <adilger@...ger.ca>
Cc:     "Theodore Y. Ts'o" <tytso@....EDU>,
        Roman Anufriev <dotdot@...dex-team.ru>,
        linux-ext4 <linux-ext4@...r.kernel.org>, Jan Kara <jack@...e.cz>,
        Wang Shilong <wangshilong1991@...il.com>,
        Dmitry Monakhov <dmtrmonakhov@...dex-team.ru>,
        "Darrick J. Wong" <darrick.wong@...cle.com>
Subject: Re: [PATCH] ext4: compare inode's i_projid with EXT4_DEF_PROJID
 rather than check EXT4_INODE_PROJINHERIT flag

On Thu, Dec 09, 2021 at 03:53:55PM -0700, Andreas Dilger wrote:
> On Dec 7, 2021, at 12:34 PM, Theodore Y. Ts'o <tytso@....EDU> wrote:
> > 
> > On Tue, Dec 07, 2021 at 05:05:19PM +0300, Roman Anufriev wrote:
> >>> Commit 7ddf79a10395 ("ext4: only set project inherit bit for directory")
> >>> removes EXT4_INODE_PROJINHERIT flag from regular files. This makes
> >>> ext4_statfs() output incorrect (function does not apply quota limits
> >>> on used/available space, etc) when called on dentry of regular file
> >>> with project quota enabled.
> > 
> > Under what circumstance is userspace trying to call statfs on a file
> > descriptor?
> 
> Who knows what users do?  Calling statfs() on a regular file works fine
> (returns stats for the filesystem), so I don't see why it wouldn't be
> consistent when calling statfs() on a file with projid set?
> 
> Darrick, how does XFS handle this case?  I think it makes sense to be
> consistent with that implementation, since that was the main reason to
> remove PROJINHERIT from regular files in the first place.

As far as I can tell, the existing ext4 implementation handles this
exactly the same that XFS does.  I would leave this alone on the grounds
that we don't really want inconsistent behavior.

--D

> 
> > Removing the test for EXT4_INODE_PROJINHERIT will cause
> > incorrect/misleading results being returned in the case where we have
> > a directory where a directory hierarchy is using project id's, but
> > which is *not* using PROJINHERIT.
> 
> One alternative would be to check the PROJINHERIT status of the parent
> directory after calling statfs() on the regular file?  That should
> keep the semantics for PROJINHERIT the same, but avoid inconsistent
> results if called on a regular file:
> 
> #ifdef CONFIG_QUOTA
> -	if (ext4_test_inode_flag(dentry->d_inode, EXT4_INODE_PROJINHERIT) &&
> +	if (ext4_test_inode_flag(S_ISDIR(dentry->d_inode) ? dentry->d_inode :
> +			   dentry->d_parent->d_inode, EXT4_INODE_PROJINHERIT) &&
> 	    sb_has_quota_limits_enabled(sb, PRJQUOTA))
> 		ext4_statfs_project(sb, EXT4_I(dentry->d_inode)->i_projid, buf);
> #endif
> 
> Roman, does that work for you?
> 
> Cheers, Andreas
> 
> 
> 
> 
>