Re: [PATCH 5.10] ext4: don't enable DAX on new encrypted files

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Ineffective assertion / Dead code. The `WARN_ON_ONCE` check for `EXT4_STATE_MAY_INLINE_DATA` in `ext4_set_context()` is logically unreachable because the flag is unconditionally cleared by `ext4_convert_inline_data()` just prior to the check.
--

commit 5589d62f76f800192f6a44a1efbb08cf6ce9550c
Author: Eric Biggers <ebiggers@kernel.org>
Subject: ext4: don't enable DAX on new encrypted files

This commit simplifies new encrypted inode creation by setting
EXT4_INODE_ENCRYPT earlier, ensuring S_DAX isn't mistakenly enabled and
then disabled, which broke via a prior commit.

Closes: https://lore.kernel.org/r/20260723085648.1500357-1-ojaswin@linux.ibm.com
Link: https://patch.msgid.link/20260730175212.36923-1-ebiggers@kernel.org

> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index c698dd5816c3a..c1538e61022cb 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c

[ ... ]

> @@ -1513,21 +1519,18 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
>  	 */
>  
>  	if (handle) {
> -		res = ext4_xattr_set_handle(handle, inode,
> -					    EXT4_XATTR_INDEX_ENCRYPTION,
> -					    EXT4_XATTR_NAME_ENCRYPTION_CONTEXT,
> -					    ctx, len, 0);
> -		if (!res) {
> -			ext4_set_inode_flag(inode, EXT4_INODE_ENCRYPT);
> -			ext4_clear_inode_state(inode,
> -					EXT4_STATE_MAY_INLINE_DATA);
> -			/*
> -			 * Update inode->i_flags - S_ENCRYPTED will be enabled,
> -			 * S_DAX may be disabled
> -			 */
> -			ext4_set_inode_flags(inode, false);
> -		}
> -		return res;
> +		/*
> +		 * __ext4_new_inode() should have already set the encrypt flag
> +		 * on the inode and avoided enabling inline data.
> +		 */
> +		if (WARN_ON_ONCE(!IS_ENCRYPTED(inode)))
> +			return -EINVAL;
> +		if (WARN_ON_ONCE(ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA)))
> +			return -EINVAL;

[Severity: Medium]
Is this assertion logically unreachable?

Looking at ext4_set_context(), it calls ext4_convert_inline_data(inode)
prior to evaluating this assertion:

ext4_set_context(...) {
    ...
    res = ext4_convert_inline_data(inode);
    if (res)
        return res;

    if (handle) {
        ...
        if (WARN_ON_ONCE(ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA)))
            return -EINVAL;

For newly created inodes, EXT4_INODE_INLINE_DATA is not yet set. This causes
ext4_convert_inline_data() to execute ext4_clear_inode_state(inode,
EXT4_STATE_MAY_INLINE_DATA) and return 0.

Since any new encrypted inode will execute this path, the state flag is
guaranteed to be cleared before the WARN_ON_ONCE evaluation. Will this render
the assertion completely ineffective?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827034018.136340-1-ebiggers@kernel.org?part=1