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

[Zhihao Cheng]

在 2026/10/14 14:28, sashiko-bot at 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 at 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?
> 

Hi, the insert_old_idx_znode() makes tnc_delete return immediately even 
level0_znode->child_cnt becomes zero, which could trigger following 
assertion failure in ubifs_search_zbranch(), this is an already existed 
process.
This issue is not severe because after the insert_old_idx_znode error 
occurs, UBIFS has already switched to read-only mode and issued an 
alarm. Subsequent ubifs_assert is merely printing more alarms, unless 
the mount option uses assert=panic.