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

[Zhihao Cheng <chengzhihao1@huawei.com>]

在 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>