Linux FSCRYPT development
 help / color / mirror / Atom feed
* [PATCH] ext4: don't enable DAX on new encrypted files
@ 2026-07-30 17:52 Eric Biggers
  2026-07-31 11:57 ` Jan Kara
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Eric Biggers @ 2026-07-30 17:52 UTC (permalink / raw)
  To: linux-ext4, Theodore Ts'o
  Cc: Andreas Dilger, Baokun Li, Jan Kara, Ojaswin Mujoo,
	Ritesh Harjani, Zhang Yi, Ira Weiny, linux-fscrypt, linux-kernel,
	Eric Biggers, Disha Goel, stable

Currently, when a new encrypted regular file is created, the call to
ext4_set_inode_flags(inode, init=true) in __ext4_new_inode() is made
before EXT4_INODE_ENCRYPT is set.  As a result, it can set S_DAX if the
filesystem is mounted with "-o dax=always".

EXT4_INODE_ENCRYPT then actually gets set a bit later in
__ext4_new_inode(), when it calls fscrypt_set_context() which calls
ext4_set_context().  ext4_set_context() sets EXT4_INODE_ENCRYPT and
calls ext4_set_inode_flags(inode, init=false) to set S_ENCRYPTED too.

This was intended to clear S_DAX as well.  However, this was broken by
commit 043546e46dc7 ("fs/ext4: Only change S_DAX on inode load").  This
causes data written to the file to bypass encryption, also causing
xfstests failures such as generic/548 (when "-o dax=always" is used).

Fix this by simplifying the flow by making __ext4_new_inode() set
EXT4_INODE_ENCRYPT earlier.  This makes it take effect in
ext4_set_inode_flags(inode, init=true), making S_DAX never be set.

Similarly, make EXT4_STATE_MAY_INLINE_DATA never be set in the first
place on new encrypted inodes.  Then it doesn't need to be cleared.

As a result of these simplifications, ext4_set_context() no longer needs
to change inode flags or state when 'handle != NULL'.  Remove that too.

Reported-by: Disha Goel <disgoel@linux.ibm.com>
Reported-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
Closes: https://lore.kernel.org/r/20260723085648.1500357-1-ojaswin@linux.ibm.com
Fixes: 043546e46dc7 ("fs/ext4: Only change S_DAX on inode load")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 fs/ext4/crypto.c | 40 ++++++++++++++++++++--------------------
 fs/ext4/ialloc.c |  4 ++++
 2 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/fs/ext4/crypto.c b/fs/ext4/crypto.c
index f41f320f4437..3971986de028 100644
--- a/fs/ext4/crypto.c
+++ b/fs/ext4/crypto.c
@@ -144,7 +144,13 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
 	if (inode->i_ino == EXT4_ROOT_INO)
 		return -EPERM;
 
-	if (WARN_ON_ONCE(IS_DAX(inode) && i_size_read(inode)))
+	/*
+	 * For new encrypted inodes, S_DAX is never set in the first place.
+	 *
+	 * For existing inodes, this is called only on empty directories.  ext4
+	 * never sets S_DAX on directories.
+	 */
+	if (WARN_ON_ONCE(IS_DAX(inode)))
 		return -EINVAL;
 
 	if (ext4_test_inode_flag(inode, EXT4_INODE_DAX))
@@ -163,6 +169,14 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
 	 */
 
 	if (handle) {
+		/*
+		 * __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;
 		/*
 		 * Since the inode is new it is ok to pass the
 		 * XATTR_CREATE flag. This is necessary to match the
@@ -170,21 +184,10 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
 		 * function with the credits allocated for the new
 		 * inode.
 		 */
-		res = ext4_xattr_set_handle(handle, inode,
-					    EXT4_XATTR_INDEX_ENCRYPTION,
-					    EXT4_XATTR_NAME_ENCRYPTION_CONTEXT,
-					    ctx, len, XATTR_CREATE);
-		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;
+		return ext4_xattr_set_handle(handle, inode,
+					     EXT4_XATTR_INDEX_ENCRYPTION,
+					     EXT4_XATTR_NAME_ENCRYPTION_CONTEXT,
+					     ctx, len, XATTR_CREATE);
 	}
 
 	res = dquot_initialize(inode);
@@ -205,10 +208,7 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
 				    ctx, len, 0);
 	if (!res) {
 		ext4_set_inode_flag(inode, EXT4_INODE_ENCRYPT);
-		/*
-		 * Update inode->i_flags - S_ENCRYPTED will be enabled,
-		 * S_DAX may be disabled
-		 */
+		/* Update inode->i_flags to set S_ENCRYPTED. */
 		ext4_set_inode_flags(inode, false);
 		res = ext4_mark_inode_dirty(handle, inode);
 		if (res)
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index a40cb27f8116..a5831fc536db 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -997,6 +997,8 @@ struct inode *__ext4_new_inode(struct mnt_idmap *idmap,
 		err = fscrypt_prepare_new_inode(dir, inode, &encrypt);
 		if (err)
 			goto out;
+		if (encrypt)
+			i_flags |= EXT4_ENCRYPT_FL;
 	}
 
 	err = dquot_initialize(inode);
@@ -1306,6 +1308,8 @@ struct inode *__ext4_new_inode(struct mnt_idmap *idmap,
 	ei->i_extra_isize = sbi->s_want_extra_isize;
 	ei->i_inline_off = 0;
 	if (ext4_has_feature_inline_data(sb) &&
+	    /* Encrypted inodes cannot have inline data */
+	    !(ei->i_flags & EXT4_ENCRYPT_FL) &&
 	    (!(ei->i_flags & (EXT4_DAX_FL|EXT4_EA_INODE_FL)) || S_ISDIR(mode)))
 		ext4_set_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA);
 	ret = inode;

base-commit: f5098b6bae761e346ebcd9da7f95622c04733cff
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] ext4: don't enable DAX on new encrypted files
  2026-07-30 17:52 [PATCH] ext4: don't enable DAX on new encrypted files Eric Biggers
@ 2026-07-31 11:57 ` Jan Kara
  2026-07-31 12:15 ` Ojaswin Mujoo
  2026-08-03  8:00 ` Disha Goel
  2 siblings, 0 replies; 4+ messages in thread
From: Jan Kara @ 2026-07-31 11:57 UTC (permalink / raw)
  To: Eric Biggers
  Cc: linux-ext4, Theodore Ts'o, Andreas Dilger, Baokun Li,
	Jan Kara, Ojaswin Mujoo, Ritesh Harjani, Zhang Yi, Ira Weiny,
	linux-fscrypt, linux-kernel, Disha Goel, stable

On Thu 30-07-26 10:52:12, Eric Biggers wrote:
> Currently, when a new encrypted regular file is created, the call to
> ext4_set_inode_flags(inode, init=true) in __ext4_new_inode() is made
> before EXT4_INODE_ENCRYPT is set.  As a result, it can set S_DAX if the
> filesystem is mounted with "-o dax=always".
> 
> EXT4_INODE_ENCRYPT then actually gets set a bit later in
> __ext4_new_inode(), when it calls fscrypt_set_context() which calls
> ext4_set_context().  ext4_set_context() sets EXT4_INODE_ENCRYPT and
> calls ext4_set_inode_flags(inode, init=false) to set S_ENCRYPTED too.
> 
> This was intended to clear S_DAX as well.  However, this was broken by
> commit 043546e46dc7 ("fs/ext4: Only change S_DAX on inode load").  This
> causes data written to the file to bypass encryption, also causing
> xfstests failures such as generic/548 (when "-o dax=always" is used).
> 
> Fix this by simplifying the flow by making __ext4_new_inode() set
> EXT4_INODE_ENCRYPT earlier.  This makes it take effect in
> ext4_set_inode_flags(inode, init=true), making S_DAX never be set.
> 
> Similarly, make EXT4_STATE_MAY_INLINE_DATA never be set in the first
> place on new encrypted inodes.  Then it doesn't need to be cleared.
> 
> As a result of these simplifications, ext4_set_context() no longer needs
> to change inode flags or state when 'handle != NULL'.  Remove that too.
> 
> Reported-by: Disha Goel <disgoel@linux.ibm.com>
> Reported-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> Closes: https://lore.kernel.org/r/20260723085648.1500357-1-ojaswin@linux.ibm.com
> Fixes: 043546e46dc7 ("fs/ext4: Only change S_DAX on inode load")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>

Looks good! Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/crypto.c | 40 ++++++++++++++++++++--------------------
>  fs/ext4/ialloc.c |  4 ++++
>  2 files changed, 24 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/ext4/crypto.c b/fs/ext4/crypto.c
> index f41f320f4437..3971986de028 100644
> --- a/fs/ext4/crypto.c
> +++ b/fs/ext4/crypto.c
> @@ -144,7 +144,13 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
>  	if (inode->i_ino == EXT4_ROOT_INO)
>  		return -EPERM;
>  
> -	if (WARN_ON_ONCE(IS_DAX(inode) && i_size_read(inode)))
> +	/*
> +	 * For new encrypted inodes, S_DAX is never set in the first place.
> +	 *
> +	 * For existing inodes, this is called only on empty directories.  ext4
> +	 * never sets S_DAX on directories.
> +	 */
> +	if (WARN_ON_ONCE(IS_DAX(inode)))
>  		return -EINVAL;
>  
>  	if (ext4_test_inode_flag(inode, EXT4_INODE_DAX))
> @@ -163,6 +169,14 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
>  	 */
>  
>  	if (handle) {
> +		/*
> +		 * __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;
>  		/*
>  		 * Since the inode is new it is ok to pass the
>  		 * XATTR_CREATE flag. This is necessary to match the
> @@ -170,21 +184,10 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
>  		 * function with the credits allocated for the new
>  		 * inode.
>  		 */
> -		res = ext4_xattr_set_handle(handle, inode,
> -					    EXT4_XATTR_INDEX_ENCRYPTION,
> -					    EXT4_XATTR_NAME_ENCRYPTION_CONTEXT,
> -					    ctx, len, XATTR_CREATE);
> -		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;
> +		return ext4_xattr_set_handle(handle, inode,
> +					     EXT4_XATTR_INDEX_ENCRYPTION,
> +					     EXT4_XATTR_NAME_ENCRYPTION_CONTEXT,
> +					     ctx, len, XATTR_CREATE);
>  	}
>  
>  	res = dquot_initialize(inode);
> @@ -205,10 +208,7 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
>  				    ctx, len, 0);
>  	if (!res) {
>  		ext4_set_inode_flag(inode, EXT4_INODE_ENCRYPT);
> -		/*
> -		 * Update inode->i_flags - S_ENCRYPTED will be enabled,
> -		 * S_DAX may be disabled
> -		 */
> +		/* Update inode->i_flags to set S_ENCRYPTED. */
>  		ext4_set_inode_flags(inode, false);
>  		res = ext4_mark_inode_dirty(handle, inode);
>  		if (res)
> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> index a40cb27f8116..a5831fc536db 100644
> --- a/fs/ext4/ialloc.c
> +++ b/fs/ext4/ialloc.c
> @@ -997,6 +997,8 @@ struct inode *__ext4_new_inode(struct mnt_idmap *idmap,
>  		err = fscrypt_prepare_new_inode(dir, inode, &encrypt);
>  		if (err)
>  			goto out;
> +		if (encrypt)
> +			i_flags |= EXT4_ENCRYPT_FL;
>  	}
>  
>  	err = dquot_initialize(inode);
> @@ -1306,6 +1308,8 @@ struct inode *__ext4_new_inode(struct mnt_idmap *idmap,
>  	ei->i_extra_isize = sbi->s_want_extra_isize;
>  	ei->i_inline_off = 0;
>  	if (ext4_has_feature_inline_data(sb) &&
> +	    /* Encrypted inodes cannot have inline data */
> +	    !(ei->i_flags & EXT4_ENCRYPT_FL) &&
>  	    (!(ei->i_flags & (EXT4_DAX_FL|EXT4_EA_INODE_FL)) || S_ISDIR(mode)))
>  		ext4_set_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA);
>  	ret = inode;
> 
> base-commit: f5098b6bae761e346ebcd9da7f95622c04733cff
> -- 
> 2.55.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ext4: don't enable DAX on new encrypted files
  2026-07-30 17:52 [PATCH] ext4: don't enable DAX on new encrypted files Eric Biggers
  2026-07-31 11:57 ` Jan Kara
@ 2026-07-31 12:15 ` Ojaswin Mujoo
  2026-08-03  8:00 ` Disha Goel
  2 siblings, 0 replies; 4+ messages in thread
From: Ojaswin Mujoo @ 2026-07-31 12:15 UTC (permalink / raw)
  To: Eric Biggers
  Cc: linux-ext4, Theodore Ts'o, Andreas Dilger, Baokun Li,
	Jan Kara, Ritesh Harjani, Zhang Yi, Ira Weiny, linux-fscrypt,
	linux-kernel, Disha Goel, stable

On Thu, Jul 30, 2026 at 10:52:12AM -0700, Eric Biggers wrote:
> Currently, when a new encrypted regular file is created, the call to
> ext4_set_inode_flags(inode, init=true) in __ext4_new_inode() is made
> before EXT4_INODE_ENCRYPT is set.  As a result, it can set S_DAX if the
> filesystem is mounted with "-o dax=always".
> 
> EXT4_INODE_ENCRYPT then actually gets set a bit later in
> __ext4_new_inode(), when it calls fscrypt_set_context() which calls
> ext4_set_context().  ext4_set_context() sets EXT4_INODE_ENCRYPT and
> calls ext4_set_inode_flags(inode, init=false) to set S_ENCRYPTED too.
> 
> This was intended to clear S_DAX as well.  However, this was broken by
> commit 043546e46dc7 ("fs/ext4: Only change S_DAX on inode load").  This
> causes data written to the file to bypass encryption, also causing
> xfstests failures such as generic/548 (when "-o dax=always" is used).
> 
> Fix this by simplifying the flow by making __ext4_new_inode() set
> EXT4_INODE_ENCRYPT earlier.  This makes it take effect in
> ext4_set_inode_flags(inode, init=true), making S_DAX never be set.
> 
> Similarly, make EXT4_STATE_MAY_INLINE_DATA never be set in the first
> place on new encrypted inodes.  Then it doesn't need to be cleared.
> 
> As a result of these simplifications, ext4_set_context() no longer needs
> to change inode flags or state when 'handle != NULL'.  Remove that too.
> 
> Reported-by: Disha Goel <disgoel@linux.ibm.com>
> Reported-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> Closes: https://lore.kernel.org/r/20260723085648.1500357-1-ojaswin@linux.ibm.com
> Fixes: 043546e46dc7 ("fs/ext4: Only change S_DAX on inode load")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>

Hey Eric, looks good and it passes all the failing tests in my dax
setup.

Feel free to add:

Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>

Regards,
ojaswin

> ---
>  fs/ext4/crypto.c | 40 ++++++++++++++++++++--------------------
>  fs/ext4/ialloc.c |  4 ++++
>  2 files changed, 24 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/ext4/crypto.c b/fs/ext4/crypto.c
> index f41f320f4437..3971986de028 100644
> --- a/fs/ext4/crypto.c
> +++ b/fs/ext4/crypto.c
> @@ -144,7 +144,13 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
>  	if (inode->i_ino == EXT4_ROOT_INO)
>  		return -EPERM;
>  
> -	if (WARN_ON_ONCE(IS_DAX(inode) && i_size_read(inode)))
> +	/*
> +	 * For new encrypted inodes, S_DAX is never set in the first place.
> +	 *
> +	 * For existing inodes, this is called only on empty directories.  ext4
> +	 * never sets S_DAX on directories.
> +	 */
> +	if (WARN_ON_ONCE(IS_DAX(inode)))
>  		return -EINVAL;
>  
>  	if (ext4_test_inode_flag(inode, EXT4_INODE_DAX))
> @@ -163,6 +169,14 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
>  	 */
>  
>  	if (handle) {
> +		/*
> +		 * __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;
>  		/*
>  		 * Since the inode is new it is ok to pass the
>  		 * XATTR_CREATE flag. This is necessary to match the
> @@ -170,21 +184,10 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
>  		 * function with the credits allocated for the new
>  		 * inode.
>  		 */
> -		res = ext4_xattr_set_handle(handle, inode,
> -					    EXT4_XATTR_INDEX_ENCRYPTION,
> -					    EXT4_XATTR_NAME_ENCRYPTION_CONTEXT,
> -					    ctx, len, XATTR_CREATE);
> -		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;
> +		return ext4_xattr_set_handle(handle, inode,
> +					     EXT4_XATTR_INDEX_ENCRYPTION,
> +					     EXT4_XATTR_NAME_ENCRYPTION_CONTEXT,
> +					     ctx, len, XATTR_CREATE);
>  	}
>  
>  	res = dquot_initialize(inode);
> @@ -205,10 +208,7 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
>  				    ctx, len, 0);
>  	if (!res) {
>  		ext4_set_inode_flag(inode, EXT4_INODE_ENCRYPT);
> -		/*
> -		 * Update inode->i_flags - S_ENCRYPTED will be enabled,
> -		 * S_DAX may be disabled
> -		 */
> +		/* Update inode->i_flags to set S_ENCRYPTED. */
>  		ext4_set_inode_flags(inode, false);
>  		res = ext4_mark_inode_dirty(handle, inode);
>  		if (res)
> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> index a40cb27f8116..a5831fc536db 100644
> --- a/fs/ext4/ialloc.c
> +++ b/fs/ext4/ialloc.c
> @@ -997,6 +997,8 @@ struct inode *__ext4_new_inode(struct mnt_idmap *idmap,
>  		err = fscrypt_prepare_new_inode(dir, inode, &encrypt);
>  		if (err)
>  			goto out;
> +		if (encrypt)
> +			i_flags |= EXT4_ENCRYPT_FL;
>  	}
>  
>  	err = dquot_initialize(inode);
> @@ -1306,6 +1308,8 @@ struct inode *__ext4_new_inode(struct mnt_idmap *idmap,
>  	ei->i_extra_isize = sbi->s_want_extra_isize;
>  	ei->i_inline_off = 0;
>  	if (ext4_has_feature_inline_data(sb) &&
> +	    /* Encrypted inodes cannot have inline data */
> +	    !(ei->i_flags & EXT4_ENCRYPT_FL) &&
>  	    (!(ei->i_flags & (EXT4_DAX_FL|EXT4_EA_INODE_FL)) || S_ISDIR(mode)))
>  		ext4_set_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA);
>  	ret = inode;
> 
> base-commit: f5098b6bae761e346ebcd9da7f95622c04733cff
> -- 
> 2.55.0
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ext4: don't enable DAX on new encrypted files
  2026-07-30 17:52 [PATCH] ext4: don't enable DAX on new encrypted files Eric Biggers
  2026-07-31 11:57 ` Jan Kara
  2026-07-31 12:15 ` Ojaswin Mujoo
@ 2026-08-03  8:00 ` Disha Goel
  2 siblings, 0 replies; 4+ messages in thread
From: Disha Goel @ 2026-08-03  8:00 UTC (permalink / raw)
  To: Eric Biggers, linux-ext4, Theodore Ts'o
  Cc: Andreas Dilger, Baokun Li, Jan Kara, Ojaswin Mujoo,
	Ritesh Harjani, Zhang Yi, Ira Weiny, linux-fscrypt, linux-kernel,
	stable

On 30/07/26 11:22 pm, Eric Biggers wrote:
> Currently, when a new encrypted regular file is created, the call to
> ext4_set_inode_flags(inode, init=true) in __ext4_new_inode() is made
> before EXT4_INODE_ENCRYPT is set.  As a result, it can set S_DAX if the
> filesystem is mounted with "-o dax=always".
> 
> EXT4_INODE_ENCRYPT then actually gets set a bit later in
> __ext4_new_inode(), when it calls fscrypt_set_context() which calls
> ext4_set_context().  ext4_set_context() sets EXT4_INODE_ENCRYPT and
> calls ext4_set_inode_flags(inode, init=false) to set S_ENCRYPTED too.
> 
> This was intended to clear S_DAX as well.  However, this was broken by
> commit 043546e46dc7 ("fs/ext4: Only change S_DAX on inode load").  This
> causes data written to the file to bypass encryption, also causing
> xfstests failures such as generic/548 (when "-o dax=always" is used).
> 
> Fix this by simplifying the flow by making __ext4_new_inode() set
> EXT4_INODE_ENCRYPT earlier.  This makes it take effect in
> ext4_set_inode_flags(inode, init=true), making S_DAX never be set.
> 
> Similarly, make EXT4_STATE_MAY_INLINE_DATA never be set in the first
> place on new encrypted inodes.  Then it doesn't need to be cleared.
> 
> As a result of these simplifications, ext4_set_context() no longer needs
> to change inode flags or state when 'handle != NULL'.  Remove that too.
> 
> Reported-by: Disha Goel <disgoel@linux.ibm.com>
> Reported-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> Closes: https://lore.kernel.org/r/20260723085648.1500357-1-ojaswin@linux.ibm.com
> Fixes: 043546e46dc7 ("fs/ext4: Only change S_DAX on inode load")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>

I have tested on a -o dax=always ext4 filesystem on ppc64le with the 
following xfstests configuration. All failing encryption tests in the
generic group now pass.

MOUNT_OPTIONS="-o block_validity,dax"
MKFS_OPTIONS="-b 65536"

Feel free to add
Tested-by: Disha Goel <disgoel@linux.ibm.com>

> ---
>   fs/ext4/crypto.c | 40 ++++++++++++++++++++--------------------
>   fs/ext4/ialloc.c |  4 ++++
>   2 files changed, 24 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/ext4/crypto.c b/fs/ext4/crypto.c
> index f41f320f4437..3971986de028 100644
> --- a/fs/ext4/crypto.c
> +++ b/fs/ext4/crypto.c
> @@ -144,7 +144,13 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
>   	if (inode->i_ino == EXT4_ROOT_INO)
>   		return -EPERM;
>   
> -	if (WARN_ON_ONCE(IS_DAX(inode) && i_size_read(inode)))
> +	/*
> +	 * For new encrypted inodes, S_DAX is never set in the first place.
> +	 *
> +	 * For existing inodes, this is called only on empty directories.  ext4
> +	 * never sets S_DAX on directories.
> +	 */
> +	if (WARN_ON_ONCE(IS_DAX(inode)))
>   		return -EINVAL;
>   
>   	if (ext4_test_inode_flag(inode, EXT4_INODE_DAX))
> @@ -163,6 +169,14 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
>   	 */
>   
>   	if (handle) {
> +		/*
> +		 * __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;
>   		/*
>   		 * Since the inode is new it is ok to pass the
>   		 * XATTR_CREATE flag. This is necessary to match the
> @@ -170,21 +184,10 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
>   		 * function with the credits allocated for the new
>   		 * inode.
>   		 */
> -		res = ext4_xattr_set_handle(handle, inode,
> -					    EXT4_XATTR_INDEX_ENCRYPTION,
> -					    EXT4_XATTR_NAME_ENCRYPTION_CONTEXT,
> -					    ctx, len, XATTR_CREATE);
> -		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;
> +		return ext4_xattr_set_handle(handle, inode,
> +					     EXT4_XATTR_INDEX_ENCRYPTION,
> +					     EXT4_XATTR_NAME_ENCRYPTION_CONTEXT,
> +					     ctx, len, XATTR_CREATE);
>   	}
>   
>   	res = dquot_initialize(inode);
> @@ -205,10 +208,7 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
>   				    ctx, len, 0);
>   	if (!res) {
>   		ext4_set_inode_flag(inode, EXT4_INODE_ENCRYPT);
> -		/*
> -		 * Update inode->i_flags - S_ENCRYPTED will be enabled,
> -		 * S_DAX may be disabled
> -		 */
> +		/* Update inode->i_flags to set S_ENCRYPTED. */
>   		ext4_set_inode_flags(inode, false);
>   		res = ext4_mark_inode_dirty(handle, inode);
>   		if (res)
> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> index a40cb27f8116..a5831fc536db 100644
> --- a/fs/ext4/ialloc.c
> +++ b/fs/ext4/ialloc.c
> @@ -997,6 +997,8 @@ struct inode *__ext4_new_inode(struct mnt_idmap *idmap,
>   		err = fscrypt_prepare_new_inode(dir, inode, &encrypt);
>   		if (err)
>   			goto out;
> +		if (encrypt)
> +			i_flags |= EXT4_ENCRYPT_FL;
>   	}
>   
>   	err = dquot_initialize(inode);
> @@ -1306,6 +1308,8 @@ struct inode *__ext4_new_inode(struct mnt_idmap *idmap,
>   	ei->i_extra_isize = sbi->s_want_extra_isize;
>   	ei->i_inline_off = 0;
>   	if (ext4_has_feature_inline_data(sb) &&
> +	    /* Encrypted inodes cannot have inline data */
> +	    !(ei->i_flags & EXT4_ENCRYPT_FL) &&
>   	    (!(ei->i_flags & (EXT4_DAX_FL|EXT4_EA_INODE_FL)) || S_ISDIR(mode)))
>   		ext4_set_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA);
>   	ret = inode;
> 
> base-commit: f5098b6bae761e346ebcd9da7f95622c04733cff

-- 
Regards,
Disha


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-03  8:01 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-30 17:52 [PATCH] ext4: don't enable DAX on new encrypted files Eric Biggers
2026-07-31 11:57 ` Jan Kara
2026-07-31 12:15 ` Ojaswin Mujoo
2026-08-03  8:00 ` Disha Goel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox