Re: [PATCH 1/2] ubifs: tnc_delete: Don't free zero-branch znodes if memory allocation fails

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ubifs: assertion failure on lookups due to zero-child znode after tnc_delete() error
--

commit 5c1854483c2d06b6fd3fc46791b422fcd958956f
Author: Zhihao Cheng <chengzhihao1@huawei.com>

ubifs: tnc_delete: Don't free zero-branch znodes if memory allocation fails

This commit delays the freeing of zero-branch znodes in tnc_delete()
until after all insert_old_idx_znode() calls succeed. It introduces a
local stack list to track znodes to be freed, aiming to prevent a
use-after-free race condition when traversing the TNC tree.

> diff --git a/fs/ubifs/tnc.c b/fs/ubifs/tnc.c
> --- a/fs/ubifs/tnc.c
> +++ b/fs/ubifs/tnc.c
[ ... ]
> @@ -2542,15 +2543,20 @@ static int tnc_delete(struct ubifs_info *c, struct ubifs_znode *znode, int n)
>  		atomic_long_dec(&c->dirty_zn_cnt);
>  
>  		err = insert_old_idx_znode(c, znode);
> -		if (err)
> +		if (err) {
> +			zn = free_head.fnext;
> +			while (zn) {
> +				struct ubifs_znode *next = zn->fnext;
> +
> +				zn->fnext = NULL;
> +				zn = next;
> +			}
>  			return err;

[Severity: High]
If insert_old_idx_znode() returns an error like -ENOMEM, does this early 
return leave the zero-child znode linked in the TNC tree?

Because we return before removing the znode from its parent's zbranch array, 
the tree retains a node with child_cnt == 0. If a subsequent read operation 
(which is permitted even after the filesystem is forced to read-only mode) 
traverses this znode, it will call ubifs_search_zbranch():

fs/ubifs/tnc_misc.c:ubifs_search_zbranch() {
        int beg = 0, end = znode->child_cnt, mid;
        int cmp;
        const struct ubifs_zbranch *zbr = &znode->zbranch[0];

        ubifs_assert(c, end > beg);
...
}

With end == 0 and beg == 0, this will trigger the assertion failure, which 
can panic the kernel if mounted with assert=panic. Does this error path need 
to remove the znode from its parent's zbranch, or otherwise mark it to 
prevent lookups from tripping the assertion?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014060252.3531356-1-chengzhihao1@huawei.com?part=1

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/