Two bug fix commit fixes in the ubi_resize_volume() were fixed by a patch in the mailing list

Two bug fix commit fixes in the ubi_resize_volume() were fixed by a patch in the mailing list

[ZhaoLong Wang]

Hi

    Mainline fix commit 1e591ea072df ("ubi: Fix unreferenced object reported by kmemleak in ubi_resize_volume()")

and 9af31d6ec1a4 ("ubi: Fix use-after-free when volume resizing failed")  involve fixing memory security issues, which

were fixed by a patch [1] ("ubi: fix slab-out-of-bounds in ubi_eba_get_ldesc+0xfb/0x130") that was on the mailing list

in 2022. In addition to fixing the race issue, I think this fix keeping old_eba_tbl might be a better solution to the UAF

problem.


    I'd like to know why patch[1] didn't get into the mainline.

[1] http://patchwork.ozlabs.org/project/linux-mtd/patch/20220124024056.1996763-1-guoxuenan@huawei.com/

Best Regards,

ZhaoLong Wang

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

Re: Two bug fix commit fixes in the ubi_resize_volume() were fixed by a patch in the mailing list

[Zhihao Cheng]

Hi,
> Hi
> 
>      Mainline fix commit 1e591ea072df ("ubi: Fix unreferenced object reported by kmemleak in ubi_resize_volume()")
> 
> and 9af31d6ec1a4 ("ubi: Fix use-after-free when volume resizing failed")  involve fixing memory security issues, which
> 
> were fixed by a patch [1] ("ubi: fix slab-out-of-bounds in ubi_eba_get_ldesc+0xfb/0x130") that was on the mailing list
> 
> in 2022. In addition to fixing the race issue, I think this fix keeping old_eba_tbl might be a better solution to the UAF
> 
> problem.
> 
> 
>      I'd like to know why patch[1] didn't get into the mainline.
> 
> [1] http://patchwork.ozlabs.org/project/linux-mtd/patch/20220124024056.1996763-1-guoxuenan@huawei.com/

I find there were three problems in ubi_resize_volume():

1. Memleak  - fixed by 1e591ea072df ("ubi: Fix unreferenced object 
reported by kmemleak in ubi_resize_volume()")
2. UAF in error handling path  - fixed by 9af31d6ec1a4 ("ubi: Fix 
use-after-free when volume resizing failed")
3. UAF in concurrent shring volume and writing 
fastmap(vol->reserved_pebs iteration)  - fixed by [1]
4. Potentional data lost in failed shrinking(failed after unmapping 
lebs)  - mentioned in [1], which is not a big problem, we can add some 
comments to explain it.
5. Too many lebs used if expanding volume failed after [1] applied:
If we update vol->reserved_pebs together with vol->eba_tbl, then other 
writing process could take lnum bigger than old vol->reserved_pebs. 
There will be zombie logical pebs(lnum greater than vol->reserved_pebs, 
could not be accessed or reclaimed) if resizing failed.
Maybe we should fix that by holding 'leb_write_lock' while expanding volume?
6. In error handling path 'out_acc', UBI should recover 'ubi->rsvd_pebs'
and 'ubi->avail_pebs' in 'pebs > 0' case, otherwise UBI will display 
wrong available peb count.

Richard, How do you think?

[1] 
http://patchwork.ozlabs.org/project/linux-mtd/patch/20220124024056.1996763-1-guoxuenan@huawei.com/

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

Re: Two bug fix commit fixes in the ubi_resize_volume() were fixed by a patch in the mailing list

[Zhihao Cheng]

> Hi,
>> Hi
>>
>>      Mainline fix commit 1e591ea072df ("ubi: Fix unreferenced object 
>> reported by kmemleak in ubi_resize_volume()")
>>
>> and 9af31d6ec1a4 ("ubi: Fix use-after-free when volume resizing 
>> failed")  involve fixing memory security issues, which
>>
>> were fixed by a patch [1] ("ubi: fix slab-out-of-bounds in 
>> ubi_eba_get_ldesc+0xfb/0x130") that was on the mailing list
>>
>> in 2022. In addition to fixing the race issue, I think this fix 
>> keeping old_eba_tbl might be a better solution to the UAF
>>
>> problem.
>>
>>
>>      I'd like to know why patch[1] didn't get into the mainline.
>>
>> [1] 
>> http://patchwork.ozlabs.org/project/linux-mtd/patch/20220124024056.1996763-1-guoxuenan@huawei.com/ 
>>
> 
> I find there were three problems in ubi_resize_volume():
> 
> 1. Memleak  - fixed by 1e591ea072df ("ubi: Fix unreferenced object 
> reported by kmemleak in ubi_resize_volume()")
> 2. UAF in error handling path  - fixed by 9af31d6ec1a4 ("ubi: Fix 
> use-after-free when volume resizing failed")
> 3. UAF in concurrent shring volume and writing 
> fastmap(vol->reserved_pebs iteration)  - fixed by [1]
> 4. Potentional data lost in failed shrinking(failed after unmapping 
> lebs)  - mentioned in [1], which is not a big problem, we can add some 
> comments to explain it.
> 5. Too many lebs used if expanding volume failed after [1] applied:
> If we update vol->reserved_pebs together with vol->eba_tbl, then other 
> writing process could take lnum bigger than old vol->reserved_pebs. 
> There will be zombie logical pebs(lnum greater than vol->reserved_pebs, 
> could not be accessed or reclaimed) if resizing failed.
> Maybe we should fix that by holding 'leb_write_lock' while expanding 
> volume?

Problem 5 cannot happened, ubi resizes volume with UBI_EXCLUSIVE volume 
lock, so there could not exist other process opening the same volume in 
write mode.

> 6. In error handling path 'out_acc', UBI should recover 'ubi->rsvd_pebs'
> and 'ubi->avail_pebs' in 'pebs > 0' case, otherwise UBI will display 
> wrong available peb count.
> 
> Richard, How do you think?
> 
> [1] 
> http://patchwork.ozlabs.org/project/linux-mtd/patch/20220124024056.1996763-1-guoxuenan@huawei.com/ 
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> .


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

Re: Two bug fix commit fixes in the ubi_resize_volume() were fixed by a patch in the mailing list

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "chengzhihao1" <chengzhihao1@huawei.com>
>>      I'd like to know why patch[1] didn't get into the mainline.
>>
>> [1] http://patchwork.ozlabs.org/project/linux-mtd/patch/20220124024056.1996763-1-guoxuenan@huawei.com/

Sorry, it fell through the cracks.

> I find there were three problems in ubi_resize_volume():
> 
> 1. Memleak  - fixed by 1e591ea072df ("ubi: Fix unreferenced object
> reported by kmemleak in ubi_resize_volume()")

Agreed.

> 2. UAF in error handling path  - fixed by 9af31d6ec1a4 ("ubi: Fix
> use-after-free when volume resizing failed")

Agreed.

> 3. UAF in concurrent shring volume and writing
> fastmap(vol->reserved_pebs iteration)  - fixed by [1]
> 4. Potentional data lost in failed shrinking(failed after unmapping
> lebs)  - mentioned in [1], which is not a big problem, we can add some
> comments to explain it.
> 5. Too many lebs used if expanding volume failed after [1] applied:
> If we update vol->reserved_pebs together with vol->eba_tbl, then other
> writing process could take lnum bigger than old vol->reserved_pebs.
> There will be zombie logical pebs(lnum greater than vol->reserved_pebs,
> could not be accessed or reclaimed) if resizing failed.
> Maybe we should fix that by holding 'leb_write_lock' while expanding volume?
> 6. In error handling path 'out_acc', UBI should recover 'ubi->rsvd_pebs'
> and 'ubi->avail_pebs' in 'pebs > 0' case, otherwise UBI will display
> wrong available peb count.
> 
> Richard, How do you think?

I agree, we should apply this patch. Guo Xuenan, can you please rebase your patch
on the latest kernel and resend?
vmt.c saw some changes and the patch does not longer apply.

Thanks,
//richard

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

Re: Two bug fix commit fixes in the ubi_resize_volume() were fixed by a patch in the mailing list

[Guo Xuenan]

Hi Richard，

On 2023/4/6 4:48, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
>> Von: "chengzhihao1" <chengzhihao1@huawei.com>
>>>       I'd like to know why patch[1] didn't get into the mainline.
>>>
>>> [1] http://patchwork.ozlabs.org/project/linux-mtd/patch/20220124024056.1996763-1-guoxuenan@huawei.com/
> Sorry, it fell through the cracks.
>
>> I find there were three problems in ubi_resize_volume():
>>
>> 1. Memleak  - fixed by 1e591ea072df ("ubi: Fix unreferenced object
>> reported by kmemleak in ubi_resize_volume()")
> Agreed.
>
>> 2. UAF in error handling path  - fixed by 9af31d6ec1a4 ("ubi: Fix
>> use-after-free when volume resizing failed")
> Agreed.
>
>> 3. UAF in concurrent shring volume and writing
>> fastmap(vol->reserved_pebs iteration)  - fixed by [1]
>> 4. Potentional data lost in failed shrinking(failed after unmapping
>> lebs)  - mentioned in [1], which is not a big problem, we can add some
>> comments to explain it.
>> 5. Too many lebs used if expanding volume failed after [1] applied:
>> If we update vol->reserved_pebs together with vol->eba_tbl, then other
>> writing process could take lnum bigger than old vol->reserved_pebs.
>> There will be zombie logical pebs(lnum greater than vol->reserved_pebs,
>> could not be accessed or reclaimed) if resizing failed.
>> Maybe we should fix that by holding 'leb_write_lock' while expanding volume?
>> 6. In error handling path 'out_acc', UBI should recover 'ubi->rsvd_pebs'
>> and 'ubi->avail_pebs' in 'pebs > 0' case, otherwise UBI will display
>> wrong available peb count.
>>
>> Richard, How do you think?
> I agree, we should apply this patch. Guo Xuenan, can you please rebase your patch
> on the latest kernel and resend?
> vmt.c saw some changes and the patch does not longer apply.
OK，I will ask my colleagues Zhaolong Wang to help me rebase and resend 
it as soon as
possible，he is currently more focused on ubifs :)

Thanks
Xuenan
> Thanks,
> //richard
>


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

Re: Two bug fix commit fixes in the ubi_resize_volume() were fixed by a patch in the mailing list

[ZhaoLong Wang]

OK, I'll try to send a series later to fix these problems in the 
ubi_resize_volume().

Thanks for everyone.

ZhaoLong Wang

在 2023/4/6 9:20, Guo Xuenan 写道:
> Hi Richard，
>
> On 2023/4/6 4:48, Richard Weinberger wrote:
>> ----- Ursprüngliche Mail -----
>>> Von: "chengzhihao1" <chengzhihao1@huawei.com>
>>>>       I'd like to know why patch[1] didn't get into the mainline.
>>>>
>>>> [1] 
>>>> http://patchwork.ozlabs.org/project/linux-mtd/patch/20220124024056.1996763-1-guoxuenan@huawei.com/
>> Sorry, it fell through the cracks.
>>
>>> I find there were three problems in ubi_resize_volume():
>>>
>>> 1. Memleak  - fixed by 1e591ea072df ("ubi: Fix unreferenced object
>>> reported by kmemleak in ubi_resize_volume()")
>> Agreed.
>>
>>> 2. UAF in error handling path  - fixed by 9af31d6ec1a4 ("ubi: Fix
>>> use-after-free when volume resizing failed")
>> Agreed.
>>
>>> 3. UAF in concurrent shring volume and writing
>>> fastmap(vol->reserved_pebs iteration)  - fixed by [1]
>>> 4. Potentional data lost in failed shrinking(failed after unmapping
>>> lebs)  - mentioned in [1], which is not a big problem, we can add some
>>> comments to explain it.
>>> 5. Too many lebs used if expanding volume failed after [1] applied:
>>> If we update vol->reserved_pebs together with vol->eba_tbl, then other
>>> writing process could take lnum bigger than old vol->reserved_pebs.
>>> There will be zombie logical pebs(lnum greater than vol->reserved_pebs,
>>> could not be accessed or reclaimed) if resizing failed.
>>> Maybe we should fix that by holding 'leb_write_lock' while expanding 
>>> volume?
>>> 6. In error handling path 'out_acc', UBI should recover 
>>> 'ubi->rsvd_pebs'
>>> and 'ubi->avail_pebs' in 'pebs > 0' case, otherwise UBI will display
>>> wrong available peb count.
>>>
>>> Richard, How do you think?
>> I agree, we should apply this patch. Guo Xuenan, can you please 
>> rebase your patch
>> on the latest kernel and resend?
>> vmt.c saw some changes and the patch does not longer apply.
> OK，I will ask my colleagues Zhaolong Wang to help me rebase and resend 
> it as soon as
> possible，he is currently more focused on ubifs :)
>
> Thanks
> Xuenan
>> Thanks,
>> //richard
>>
>

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