[PATCH] ubifs: fix host inode TNC offset when deleting an xattr

[PATCH] ubifs: fix host inode TNC offset when deleting an xattr

[Nan Lin]

When ubifs_jnl_delete_xattr() deletes an extended attribute, it writes
a group of three nodes to the base head: the xent node, the xattr
inode node and the host inode node. The host inode node is actually
packed at xent_offs + aligned_xlen + UBIFS_INO_NODE_SZ, but the
position registered into TNC is computed backwards as
xent_offs + len - hlen. The two differ by the group tail padding,
ALIGN(hlen, 8) - hlen.

Whenever the host inode's data_len is not a multiple of 8, the offset
registered in TNC points 1 to 7 bytes past the real node header. The
wrong offset gets committed into the on-flash index, and on the next
mount read_znode() fails the offs & 7 alignment check, returns -EINVAL
and switches the filesystem to read-only mode, reporting
"bad indexing node ... error 2".

Which inodes are affected depends on data_len:
 - block/character devices always carry data_len = 4 and are always
   affected;
 - symlinks carry data_len = strlen(target) and are affected unless
   the target length happens to be a multiple of 8;
 - regular files, FIFOs and sockets carry data_len = 0 and are
   unaffected.

The bug reproduces by setting an xattr on a symlink with attr -Rs and
removing it with attr -Rr: after unmount and remount, readlink()
fails, the index node is rejected with error 2 (bad branch offset) and
the filesystem turns read-only.

Fix the offset computation by accumulating the group layout forward
instead of subtracting from the total length: register the host inode
at xent_offs + aligned_xlen + UBIFS_INO_NODE_SZ, which matches the
packed position exactly. The host node length hlen is already correct
and stays unchanged.

Signed-off-by: Nan Lin <lin.nan@h3c.com>
---
 fs/ubifs/journal.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
index 40a95a2fad50..1a32d641e658 100644
--- a/fs/ubifs/journal.c
+++ b/fs/ubifs/journal.c
@@ -1943,7 +1943,9 @@ int ubifs_jnl_delete_xattr(struct ubifs_info *c, const struct inode *host,
 
 	/* And update TNC with the new host inode position */
 	ino_key_init(c, &key1, host->i_ino);
-	err = ubifs_tnc_add(c, &key1, lnum, xent_offs + len - hlen, hlen, hash);
+	err = ubifs_tnc_add(c, &key1, lnum,
+				xent_offs + aligned_xlen + UBIFS_INO_NODE_SZ,
+				hlen, hash);
 	if (err)
 		goto out_ro;
 
-- 
2.33.0

AW: [PATCH] ubifs: fix host inode TNC offset when deleting an xattr

[Richard Weinberger]

Am Mon, Oct 12, 2026, 09:58 schrieb Nan Lin <lin.nan@h3c.com>:
> When ubifs_jnl_delete_xattr() deletes an extended attribute, it writes
> a group of three nodes to the base head: the xent node, the xattr
> inode node and the host inode node. The host inode node is actually
> packed at xent_offs + aligned_xlen + UBIFS_INO_NODE_SZ, but the
> position registered into TNC is computed backwards as
> xent_offs + len - hlen. The two differ by the group tail padding,
> ALIGN(hlen, 8) - hlen.
> 
> Whenever the host inode's data_len is not a multiple of 8, the offset
> registered in TNC points 1 to 7 bytes past the real node header. The
> wrong offset gets committed into the on-flash index, and on the next
> mount read_znode() fails the offs & 7 alignment check, returns -EINVAL
> and switches the filesystem to read-only mode, reporting
> "bad indexing node ... error 2".
> 
> Which inodes are affected depends on data_len:
>  - block/character devices always carry data_len = 4 and are always
>    affected;
>  - symlinks carry data_len = strlen(target) and are affected unless
>    the target length happens to be a multiple of 8;
>  - regular files, FIFOs and sockets carry data_len = 0 and are
>    unaffected.
> 
> The bug reproduces by setting an xattr on a symlink with attr -Rs and
> removing it with attr -Rr: after unmount and remount, readlink()
> fails, the index node is rejected with error 2 (bad branch offset) and
> the filesystem turns read-only.
> 
> Fix the offset computation by accumulating the group layout forward
> instead of subtracting from the total length: register the host inode
> at xent_offs + aligned_xlen + UBIFS_INO_NODE_SZ, which matches the
> packed position exactly. The host node length hlen is already correct
> and stays unchanged.

Fixes: 1e51764a3c2a ("UBIFS: add new flash file system")
Cc: stable@vger.kernel.org

> Signed-off-by: Nan Lin <lin.nan@h3c.com>
> ---
>  fs/ubifs/journal.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Good catch!

Please ask your LLM to write less wordy commit messages.
How about something like that:

ubifs: fix host inode TNC offset when deleting an xattr

ubifs_jnl_delete_xattr() writes a group of three nodes: the xent, the
xattr inode and the host inode. The host inode is packed at
xent_offs + aligned_xlen + UBIFS_INO_NODE_SZ, but its TNC position is
computed backwards as xent_offs + len - hlen. Since len includes the
group's tail padding, ALIGN(hlen, 8) - hlen, the two differ whenever
the host inode's inline data length is not a multiple of 8. This
affects symlinks and device nodes.

The unaligned offset ends up in the on-flash index and the next mount
rejects the index node, turning the filesystem read-only.

> diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
> index 40a95a2fad50..1a32d641e658 100644
> --- a/fs/ubifs/journal.c
> +++ b/fs/ubifs/journal.c
> @@ -1943,7 +1943,9 @@ int ubifs_jnl_delete_xattr(struct ubifs_info *c, const struct inode *host,
>  
>  	/* And update TNC with the new host inode position */
>  	ino_key_init(c, &key1, host->i_ino);
> -	err = ubifs_tnc_add(c, &key1, lnum, xent_offs + len - hlen, hlen, hash);
> +	err = ubifs_tnc_add(c, &key1, lnum,
> +				xent_offs + aligned_xlen + UBIFS_INO_NODE_SZ,
> +				hlen, hash);

How about adding a new variable instead of open coding the offset?
The offsets are already super hard to understand, more magic doesn't
make it better.

e.g.
host_offs = xent_offs + aligned_xlen + UBIFS_INO_NODE_SZ;

Thanks,
//richard

Re: AW: [PATCH] ubifs: fix host inode TNC offset when deleting an xattr

[Zhihao Cheng]

在 2026/10/13 3:56, Richard Weinberger 写道:
> Am Mon, Oct 12, 2026, 09:58 schrieb Nan Lin <lin.nan@h3c.com>:
>> When ubifs_jnl_delete_xattr() deletes an extended attribute, it writes
>> a group of three nodes to the base head: the xent node, the xattr
>> inode node and the host inode node. The host inode node is actually
>> packed at xent_offs + aligned_xlen + UBIFS_INO_NODE_SZ, but the
>> position registered into TNC is computed backwards as
>> xent_offs + len - hlen. The two differ by the group tail padding,
>> ALIGN(hlen, 8) - hlen.
>>
>> Whenever the host inode's data_len is not a multiple of 8, the offset
>> registered in TNC points 1 to 7 bytes past the real node header. The
>> wrong offset gets committed into the on-flash index, and on the next
>> mount read_znode() fails the offs & 7 alignment check, returns -EINVAL
>> and switches the filesystem to read-only mode, reporting
>> "bad indexing node ... error 2".
>>
>> Which inodes are affected depends on data_len:
>>   - block/character devices always carry data_len = 4 and are always
>>     affected;
>>   - symlinks carry data_len = strlen(target) and are affected unless
>>     the target length happens to be a multiple of 8;
>>   - regular files, FIFOs and sockets carry data_len = 0 and are
>>     unaffected.
>>
>> The bug reproduces by setting an xattr on a symlink with attr -Rs and
>> removing it with attr -Rr: after unmount and remount, readlink()
>> fails, the index node is rejected with error 2 (bad branch offset) and
>> the filesystem turns read-only.
>>
>> Fix the offset computation by accumulating the group layout forward
>> instead of subtracting from the total length: register the host inode
>> at xent_offs + aligned_xlen + UBIFS_INO_NODE_SZ, which matches the
>> packed position exactly. The host node length hlen is already correct
>> and stays unchanged.
> 
> Fixes: 1e51764a3c2a ("UBIFS: add new flash file system")
> Cc: stable@vger.kernel.org
> 
>> Signed-off-by: Nan Lin <lin.nan@h3c.com>
>> ---
>>   fs/ubifs/journal.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> Good catch!
> 
> Please ask your LLM to write less wordy commit messages.
> How about something like that:
> 
> ubifs: fix host inode TNC offset when deleting an xattr
> 
> ubifs_jnl_delete_xattr() writes a group of three nodes: the xent, the
> xattr inode and the host inode. The host inode is packed at
> xent_offs + aligned_xlen + UBIFS_INO_NODE_SZ, but its TNC position is
> computed backwards as xent_offs + len - hlen. Since len includes the
> group's tail padding, ALIGN(hlen, 8) - hlen, the two differ whenever
> the host inode's inline data length is not a multiple of 8. This
> affects symlinks and device nodes.
> 
> The unaligned offset ends up in the on-flash index and the next mount
> rejects the index node, turning the filesystem read-only.
> 
>> diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
>> index 40a95a2fad50..1a32d641e658 100644
>> --- a/fs/ubifs/journal.c
>> +++ b/fs/ubifs/journal.c
>> @@ -1943,7 +1943,9 @@ int ubifs_jnl_delete_xattr(struct ubifs_info *c, const struct inode *host,
>>   
>>   	/* And update TNC with the new host inode position */
>>   	ino_key_init(c, &key1, host->i_ino);
>> -	err = ubifs_tnc_add(c, &key1, lnum, xent_offs + len - hlen, hlen, hash);
>> +	err = ubifs_tnc_add(c, &key1, lnum,
>> +				xent_offs + aligned_xlen + UBIFS_INO_NODE_SZ,
>> +				hlen, hash);
> 
> How about adding a new variable instead of open coding the offset?
> The offsets are already super hard to understand, more magic doesn't
> make it better.
> 
> e.g.
> host_offs = xent_offs + aligned_xlen + UBIFS_INO_NODE_SZ;
> 

The problem can be easily reproduced by following script:
mnt=/mnt
mknod $mnt/node c 0 0
setfattr -n security.123 -v 123 $mnt/node
setfattr -x security.123 $mnt/node
umount $mnt
mount /dev/ubi0_0 $mnt
dmesg
[60700.197045] UBIFS error (ubi0:0 pid 8770): ubifs_load_znode [ubifs]: 
bad branch 2
[60700.197352] UBIFS error (ubi0:0 pid 8770): ubifs_load_znode [ubifs]: 
bad indexing node at LEB 11:2048
[60700.197604] 	magic          0x6101831
[60700.197610] 	crc            0x374e477d
[60700.197616] 	node_type      9 (indexing node)
[60700.197622] 	group_type     0 (no node group)
[60700.197629] 	sqnum          28
[60700.197634] 	len            88
[60700.197640] 	child_cnt      3
[60700.197645] 	level          0
[60700.197650] 	Branches:
[60700.197655] 	0: LEB 12:2280 len 160 key (1, inode)
[60700.197667] 	1: LEB 12:2048 len 61 key (1, direntry, 0x1b1f5d0)
[60700.197678] 	2: LEB 12:4332 len 164 key (65, inode)
[60700.197698] UBIFS error (ubi0:0 pid 8770): ubifs_iget [ubifs]: failed 
to read inode 1, error -22

The reason we haven't discovered this issue for so long might be that 
ordinary people don't set xattrs on symlinks or device nodes.
BTW,

Tested-by: Zhihao Cheng <chengzhihao1@huawei.com>

[PATCH v2] ubifs: fix host inode TNC offset when deleting an xattr

[Nan Lin]

ubifs_jnl_delete_xattr() writes a group of three nodes: the xent, the
xattr inode and the host inode. The host inode is packed at
xent_offs + aligned_xlen + UBIFS_INO_NODE_SZ, but its TNC position is
computed backwards as xent_offs + len - hlen. Since len includes the
group's tail padding, ALIGN(hlen, 8) - hlen, the two differ whenever
the host inode's inline data length is not a multiple of 8. This
affects symlinks and device nodes.

The unaligned offset ends up in the on-flash index and the next mount
rejects the index node, turning the filesystem read-only.

Fixes: 1e51764a3c2a ("UBIFS: add new flash file system")
Cc: stable@vger.kernel.org
Tested-by: Zhihao Cheng <chengzhihao1@huawei.com>
Signed-off-by: Nan Lin <lin.nan@h3c.com>

Changes since v1:
 - compute the host inode offset into a new host_offs variable instead
   of open-coding the offset (Richard Weinberger);
 - shorten the commit message.

---
 fs/ubifs/journal.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
index 40a95a2fad50..0b2dae57e467 100644
--- a/fs/ubifs/journal.c
+++ b/fs/ubifs/journal.c
@@ -1858,7 +1858,8 @@ int ubifs_jnl_delete_xattr(struct ubifs_info *c, const struct inode *host,
 			   const struct inode *inode,
 			   const struct fscrypt_name *nm)
 {
-	int err, xlen, hlen, len, lnum, xent_offs, aligned_xlen, write_len;
+	int err, xlen, hlen, len, lnum, xent_offs, host_offs, aligned_xlen,
+		write_len;
 	struct ubifs_dent_node *xent;
 	struct ubifs_ino_node *ino;
 	union ubifs_key xent_key, key1, key2;
@@ -1915,6 +1916,8 @@ int ubifs_jnl_delete_xattr(struct ubifs_info *c, const struct inode *host,
 		ubifs_wbuf_add_ino_nolock(&c->jheads[BASEHD].wbuf, host->i_ino);
 	release_head(c, BASEHD);
 
+	host_offs = xent_offs + aligned_xlen + UBIFS_INO_NODE_SZ;
+
 	ubifs_add_auth_dirt(c, lnum);
 	kfree(xent);
 	if (err)
@@ -1943,7 +1946,7 @@ int ubifs_jnl_delete_xattr(struct ubifs_info *c, const struct inode *host,
 
 	/* And update TNC with the new host inode position */
 	ino_key_init(c, &key1, host->i_ino);
-	err = ubifs_tnc_add(c, &key1, lnum, xent_offs + len - hlen, hlen, hash);
+	err = ubifs_tnc_add(c, &key1, lnum, host_offs, hlen, hash);
 	if (err)
 		goto out_ro;
 
-- 
2.33.0

Re: [PATCH v2] ubifs: fix host inode TNC offset when deleting an xattr

[Zhihao Cheng]

在 2026/10/13 13:29, Nan Lin 写道:
> ubifs_jnl_delete_xattr() writes a group of three nodes: the xent, the
> xattr inode and the host inode. The host inode is packed at
> xent_offs + aligned_xlen + UBIFS_INO_NODE_SZ, but its TNC position is
> computed backwards as xent_offs + len - hlen. Since len includes the
> group's tail padding, ALIGN(hlen, 8) - hlen, the two differ whenever
> the host inode's inline data length is not a multiple of 8. This
> affects symlinks and device nodes.
> 
> The unaligned offset ends up in the on-flash index and the next mount
> rejects the index node, turning the filesystem read-only.
> 
> Fixes: 1e51764a3c2a ("UBIFS: add new flash file system")
> Cc: stable@vger.kernel.org
> Tested-by: Zhihao Cheng <chengzhihao1@huawei.com>
> Signed-off-by: Nan Lin <lin.nan@h3c.com>
> 
> Changes since v1:
>   - compute the host inode offset into a new host_offs variable instead
>     of open-coding the offset (Richard Weinberger);
>   - shorten the commit message.
> 
> ---
>   fs/ubifs/journal.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)

Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>
> 
> diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
> index 40a95a2fad50..0b2dae57e467 100644
> --- a/fs/ubifs/journal.c
> +++ b/fs/ubifs/journal.c
> @@ -1858,7 +1858,8 @@ int ubifs_jnl_delete_xattr(struct ubifs_info *c, const struct inode *host,
>   			   const struct inode *inode,
>   			   const struct fscrypt_name *nm)
>   {
> -	int err, xlen, hlen, len, lnum, xent_offs, aligned_xlen, write_len;
> +	int err, xlen, hlen, len, lnum, xent_offs, host_offs, aligned_xlen,
> +		write_len;
>   	struct ubifs_dent_node *xent;
>   	struct ubifs_ino_node *ino;
>   	union ubifs_key xent_key, key1, key2;
> @@ -1915,6 +1916,8 @@ int ubifs_jnl_delete_xattr(struct ubifs_info *c, const struct inode *host,
>   		ubifs_wbuf_add_ino_nolock(&c->jheads[BASEHD].wbuf, host->i_ino);
>   	release_head(c, BASEHD);
>   
> +	host_offs = xent_offs + aligned_xlen + UBIFS_INO_NODE_SZ;
> +
>   	ubifs_add_auth_dirt(c, lnum);
>   	kfree(xent);
>   	if (err)
> @@ -1943,7 +1946,7 @@ int ubifs_jnl_delete_xattr(struct ubifs_info *c, const struct inode *host,
>   
>   	/* And update TNC with the new host inode position */
>   	ino_key_init(c, &key1, host->i_ino);
> -	err = ubifs_tnc_add(c, &key1, lnum, xent_offs + len - hlen, hlen, hash);
> +	err = ubifs_tnc_add(c, &key1, lnum, host_offs, hlen, hash);
>   	if (err)
>   		goto out_ro;
>   
>