Re: [PATCH] quota: explicitly forbid quota files from being encrypted

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon 04-09-23 17:32:27, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@xxxxxxxxxx>
> 
> Since commit d7e7b9af104c ("fscrypt: stop using keyrings subsystem for
> fscrypt_master_key"), xfstest generic/270 causes a WARNING when run on
> f2fs with test_dummy_encryption in the mount options:
> 
> $ kvm-xfstests -c f2fs/encrypt generic/270
> [...]
> WARNING: CPU: 1 PID: 2453 at fs/crypto/keyring.c:240 fscrypt_destroy_keyring+0x1f5/0x260
> 
> The cause of the WARNING is that not all encrypted inodes have been
> evicted before fscrypt_destroy_keyring() is called, which violates an
> assumption.  This happens because the test uses an external quota file,
> which gets automatically encrypted due to test_dummy_encryption.
> 
> Encryption of quota files has never really been supported.  On ext4,
> ext4_quota_read() does not decrypt the data, so encrypted quota files
> are always considered invalid on ext4.  On f2fs, f2fs_quota_read() uses
> the pagecache, so trying to use an encrypted quota file gets farther,
> resulting in the issue described above being possible.  But this was
> never intended to be possible, and there is no use case for it.
> 
> Therefore, make the quota support layer explicitly reject using
> IS_ENCRYPTED inodes when quotaon is attempted.
> 
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Eric Biggers <ebiggers@xxxxxxxxxx>

Looks good. I'll queue this patch into my tree and send it to Linus for
RC2.

								Honza

> ---
>  fs/quota/dquot.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
> index 9e72bfe8bbad9..7e268cd2727cc 100644
> --- a/fs/quota/dquot.c
> +++ b/fs/quota/dquot.c
> @@ -2339,6 +2339,20 @@ static int vfs_setup_quota_inode(struct inode *inode, int type)
>  	if (sb_has_quota_loaded(sb, type))
>  		return -EBUSY;
>  
> +	/*
> +	 * Quota files should never be encrypted.  They should be thought of as
> +	 * filesystem metadata, not user data.  New-style internal quota files
> +	 * cannot be encrypted by users anyway, but old-style external quota
> +	 * files could potentially be incorrectly created in an encrypted
> +	 * directory, hence this explicit check.  Some reasons why encrypted
> +	 * quota files don't work include: (1) some filesystems that support
> +	 * encryption don't handle it in their quota_read and quota_write, and
> +	 * (2) cleaning up encrypted quota files at unmount would need special
> +	 * consideration, as quota files are cleaned up later than user files.
> +	 */
> +	if (IS_ENCRYPTED(inode))
> +		return -EINVAL;
> +
>  	dqopt->files[type] = igrab(inode);
>  	if (!dqopt->files[type])
>  		return -EIO;
> 
> base-commit: 708283abf896dd4853e673cc8cba70acaf9bf4ea
> -- 
> 2.42.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux