* [PATCH] ext4: don't enable DAX on new encrypted files
@ 2026-07-30 17:52 Eric Biggers
2026-07-31 11:57 ` Jan Kara
2026-07-31 12:15 ` Ojaswin Mujoo
0 siblings, 2 replies; 3+ 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] 3+ 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
1 sibling, 0 replies; 3+ 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] 3+ 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
1 sibling, 0 replies; 3+ 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] 3+ messages in thread
end of thread, other threads:[~2026-07-31 12:16 UTC | newest]
Thread overview: 3+ 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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox