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
