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@xxxxxxx>:
> 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@xxxxxxxxxxxxxxx

> Signed-off-by: Nan Lin <lin.nan@xxxxxxx>
> ---
>  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