[PATCH 2/3] ubifs: xattr: preserve the encryption flag when creation fails

[PATCH 2/3] ubifs: xattr: preserve the encryption flag when creation fails

[shao.mingyin]

create_xattr() clears UBIFS_CRYPT_FL in its error path regardless of
whether this call set the flag.  The flag is also set for every inode
that already carries an encryption context, so a failed setxattr() on
such an inode clears it from the in-memory inode.  The next inode write
then packs ui->flags without UBIFS_CRYPT_FL, and after the next mount
the inode is no longer treated as encrypted although its names and data
still are.  ubifs_jnl_update() can fail before anything is written, for
example when its journal buffer cannot be allocated.

Remember whether this call actually set the flag and only clear it in
that case.

Fixes: d475a507457b ("ubifs: Add skeleton for fscrypto")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
diff --git a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c
index 3b0e8a270f59..7f82ddb9e3f2 100644
--- a/fs/ubifs/xattr.c
+++ b/fs/ubifs/xattr.c
@@ -69,6 +69,7 @@ static int create_xattr(struct ubifs_info *c, struct inode *host,
 			const struct fscrypt_name *nm, const void *value, int size)
 {
 	int err, names_len;
+	bool set_crypt = false;
 	struct inode *inode;
 	struct ubifs_inode *ui, *host_ui = ubifs_inode(host);
 	struct ubifs_budget_req req = { .new_ino = 1, .new_dent = 1,
@@ -133,8 +134,10 @@ static int create_xattr(struct ubifs_info *c, struct inode *host,
 	 * To avoid multiple updates of the same inode in the same operation,
 	 * let's do it here.
 	 */
-	if (strcmp(fname_name(nm), UBIFS_XATTR_NAME_ENCRYPTION_CONTEXT) == 0)
+	if (strcmp(fname_name(nm), UBIFS_XATTR_NAME_ENCRYPTION_CONTEXT) == 0) {
+		set_crypt = !(host_ui->flags & UBIFS_CRYPT_FL);
 		host_ui->flags |= UBIFS_CRYPT_FL;
+	}
 
 	err = ubifs_jnl_update(c, host, nm, inode, 0, 1, 0);
 	if (err)
@@ -152,7 +155,8 @@ static int create_xattr(struct ubifs_info *c, struct inode *host,
 	host_ui->xattr_size -= CALC_DENT_SIZE(fname_len(nm));
 	host_ui->xattr_size -= CALC_XATTR_BYTES(size);
 	host_ui->xattr_names -= fname_len(nm);
-	host_ui->flags &= ~UBIFS_CRYPT_FL;
+	if (set_crypt)
+		host_ui->flags &= ~UBIFS_CRYPT_FL;
 	mutex_unlock(&host_ui->ui_mutex);
 out_free:
 	make_bad_inode(inode);

Re: [PATCH 2/3] ubifs: xattr: preserve the encryption flag when creation fails

[Zhihao Cheng]

在 2026/11/2 19:56, shao.mingyin@zte.com.cn 写道:
> create_xattr() clears UBIFS_CRYPT_FL in its error path regardless of
> whether this call set the flag.  The flag is also set for every inode
> that already carries an encryption context, so a failed setxattr() on
> such an inode clears it from the in-memory inode.  The next inode write
> then packs ui->flags without UBIFS_CRYPT_FL, and after the next mount
> the inode is no longer treated as encrypted although its names and data
> still are.  ubifs_jnl_update() can fail before anything is written, for
> example when its journal buffer cannot be allocated.
> 
> Remember whether this call actually set the flag and only clear it in
> that case.
> 
> Fixes: d475a507457b ("ubifs: Add skeleton for fscrypto")
> Cc: stable@vger.kernel.org
> Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
> ---
Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>
> diff --git a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c
> index 3b0e8a270f59..7f82ddb9e3f2 100644
> --- a/fs/ubifs/xattr.c
> +++ b/fs/ubifs/xattr.c
> @@ -69,6 +69,7 @@ static int create_xattr(struct ubifs_info *c, struct inode *host,
>   			const struct fscrypt_name *nm, const void *value, int size)
>   {
>   	int err, names_len;
> +	bool set_crypt = false;
>   	struct inode *inode;
>   	struct ubifs_inode *ui, *host_ui = ubifs_inode(host);
>   	struct ubifs_budget_req req = { .new_ino = 1, .new_dent = 1,
> @@ -133,8 +134,10 @@ static int create_xattr(struct ubifs_info *c, struct inode *host,
>   	 * To avoid multiple updates of the same inode in the same operation,
>   	 * let's do it here.
>   	 */
> -	if (strcmp(fname_name(nm), UBIFS_XATTR_NAME_ENCRYPTION_CONTEXT) == 0)
> +	if (strcmp(fname_name(nm), UBIFS_XATTR_NAME_ENCRYPTION_CONTEXT) == 0) {
> +		set_crypt = !(host_ui->flags & UBIFS_CRYPT_FL);
>   		host_ui->flags |= UBIFS_CRYPT_FL;
> +	}
>   
>   	err = ubifs_jnl_update(c, host, nm, inode, 0, 1, 0);
>   	if (err)
> @@ -152,7 +155,8 @@ static int create_xattr(struct ubifs_info *c, struct inode *host,
>   	host_ui->xattr_size -= CALC_DENT_SIZE(fname_len(nm));
>   	host_ui->xattr_size -= CALC_XATTR_BYTES(size);
>   	host_ui->xattr_names -= fname_len(nm);
> -	host_ui->flags &= ~UBIFS_CRYPT_FL;
> +	if (set_crypt)
> +		host_ui->flags &= ~UBIFS_CRYPT_FL;
>   	mutex_unlock(&host_ui->ui_mutex);
>   out_free:
>   	make_bad_inode(inode);
> .
> 

[PATCH 0/3] ubifs: fix an auth node leak and two error-handling bugs

[shao.mingyin]

This series fixes three independent problems in ubifs, all found while
auditing the error paths around the GC write-buffer, xattr creation and
authenticated node hashing.

Patch 1 fixes a leak.  move_nodes() allocates the authentication node for
the GC journal head and writes it into the GC write-buffer; the buffer is
freed only when one of the steps fails.  On an authenticated file system
every GC pass that moves nodes therefore leaks ubifs_auth_node_sz()
bytes, so the loss is unbounded on a long-running system.

Patch 2 fixes state that survives a failed operation.  create_xattr()
sets UBIFS_CRYPT_FL for inodes that carry an encryption context, but its
error path clears the flag whether this call set it or not.  A setxattr()
that fails before anything reaches the journal, for example because its
journal buffer cannot be allocated, therefore leaves the in-memory inode
marked as unencrypted; the next inode write persists that, and after the
next mount the inode is no longer treated as encrypted although its names
and data still are.

Patch 3 stops five call sites from ignoring ubifs_node_calc_hash().  With
authentication enabled a failing crypto operation leaves those callers
using an uninitialised stack buffer as the node hash; it is then stored in
the TNC or in the master node, and the affected nodes fail verification
later.

The three patches are independent and can be applied in any order or as a
subset.  They are listed by severity, not by dependency: the leak first,
then the persistent state loss, then the error propagation.

All three carry Fixes: and Cc: stable.

Testing: the patches apply to next-20261030, pass checkpatch.pl --strict
with 0 errors/0 warnings/0 checks, and fs/ubifs/ builds with
CONFIG_UBIFS_FS_AUTHENTICATION, CONFIG_UBIFS_FS_XATTR and
CONFIG_UBIFS_FS_SECURITY enabled and W=1 without new warnings.  They were
not exercised at runtime: this machine has no UBI/MTD tooling or
emulation, so there is no mount test.  Patch 1 is the easiest to verify by
inspection; patch 2 needs an allocation or I/O failure in
ubifs_jnl_update(); patch 3 needs a crypto failure inside
ubifs_node_calc_hash().

Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c

[PATCH 2/3] ubifs: xattr: preserve the encryption flag when creation fails

[shao.mingyin]

create_xattr() clears UBIFS_CRYPT_FL in its error path regardless of
whether this call set the flag.  The flag is also set for every inode
that already carries an encryption context, so a failed setxattr() on
such an inode clears it from the in-memory inode.  The next inode write
then packs ui->flags without UBIFS_CRYPT_FL, and after the next mount
the inode is no longer treated as encrypted although its names and data
still are.  ubifs_jnl_update() can fail before anything is written, for
example when its journal buffer cannot be allocated.

Remember whether this call actually set the flag and only clear it in
that case.

Fixes: d475a507457b ("ubifs: Add skeleton for fscrypto")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
diff --git a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c
index 3b0e8a270f59..7f82ddb9e3f2 100644
--- a/fs/ubifs/xattr.c
+++ b/fs/ubifs/xattr.c
@@ -69,6 +69,7 @@ static int create_xattr(struct ubifs_info *c, struct inode *host,
 			const struct fscrypt_name *nm, const void *value, int size)
 {
 	int err, names_len;
+	bool set_crypt = false;
 	struct inode *inode;
 	struct ubifs_inode *ui, *host_ui = ubifs_inode(host);
 	struct ubifs_budget_req req = { .new_ino = 1, .new_dent = 1,
@@ -133,8 +134,10 @@ static int create_xattr(struct ubifs_info *c, struct inode *host,
 	 * To avoid multiple updates of the same inode in the same operation,
 	 * let's do it here.
 	 */
-	if (strcmp(fname_name(nm), UBIFS_XATTR_NAME_ENCRYPTION_CONTEXT) == 0)
+	if (strcmp(fname_name(nm), UBIFS_XATTR_NAME_ENCRYPTION_CONTEXT) == 0) {
+		set_crypt = !(host_ui->flags & UBIFS_CRYPT_FL);
 		host_ui->flags |= UBIFS_CRYPT_FL;
+	}

 	err = ubifs_jnl_update(c, host, nm, inode, 0, 1, 0);
 	if (err)
@@ -152,7 +155,8 @@ static int create_xattr(struct ubifs_info *c, struct inode *host,
 	host_ui->xattr_size -= CALC_DENT_SIZE(fname_len(nm));
 	host_ui->xattr_size -= CALC_XATTR_BYTES(size);
 	host_ui->xattr_names -= fname_len(nm);
-	host_ui->flags &= ~UBIFS_CRYPT_FL;
+	if (set_crypt)
+		host_ui->flags &= ~UBIFS_CRYPT_FL;
 	mutex_unlock(&host_ui->ui_mutex);
 out_free:
 	make_bad_inode(inode);