[PATCH 0/2] Fix use-after-free and kmemleak in ubi_resize_volume()

[PATCH 0/2] Fix use-after-free and kmemleak in ubi_resize_volume()

[Li Zetao]

This patchset fixes two memory usage issues.

Patch 1 resolves the use-after-free issue, this is happening in volume 
resizing failed. In volume resizing process, the old eba table will be
replaced by the new. But on error handing patch, the old eba will be 
freed, which causing an use-after-free fault when resizing volume
next time.

Patch 2 resolves the kmemleak issue, this is also happening in volume
resizing failed. "new_eba_tbl" is created by ubi_eba_create_table() 
but destroyed by kfree().

Li Zetao (2):
  ubi: Fix use-after-free when volume resizing failed
  ubi: Fix unreferenced object reported by kmemleak in
    ubi_resize_volume()

 drivers/mtd/ubi/vmt.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

-- 
2.31.1


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

[PATCH 1/2] ubi: Fix use-after-free when volume resizing failed

[Li Zetao]

There is an use-after-free problem reported by KASAN:
  ==================================================================
  BUG: KASAN: use-after-free in ubi_eba_copy_table+0x11f/0x1c0 [ubi]
  Read of size 8 at addr ffff888101eec008 by task ubirsvol/4735

  CPU: 2 PID: 4735 Comm: ubirsvol
  Not tainted 6.1.0-rc1-00003-g84fa3304a7fc-dirty #14
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
  BIOS 1.14.0-1.fc33 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x34/0x44
   print_report+0x171/0x472
   kasan_report+0xad/0x130
   ubi_eba_copy_table+0x11f/0x1c0 [ubi]
   ubi_resize_volume+0x4f9/0xbc0 [ubi]
   ubi_cdev_ioctl+0x701/0x1850 [ubi]
   __x64_sys_ioctl+0x11d/0x170
   do_syscall_64+0x35/0x80
   entry_SYSCALL_64_after_hwframe+0x46/0xb0
   </TASK>

When ubi_change_vtbl_record() returns an error in ubi_resize_volume(),
"new_eba_tbl" will be freed on error handing path, but it is holded
by "vol->eba_tbl" in ubi_eba_replace_table(). It means that the liftcycle
of "vol->eba_tbl" and "vol" are different, so when resizing volume in
next time, it causing an use-after-free fault.

Fix it by not freeing "new_eba_tbl" after it replaced in
ubi_eba_replace_table(), while will be freed in next volume resizing.

Fixes: 801c135ce73d ("UBI: Unsorted Block Images")
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
 drivers/mtd/ubi/vmt.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/ubi/vmt.c b/drivers/mtd/ubi/vmt.c
index 8fcc0bdf0635..74637575346e 100644
--- a/drivers/mtd/ubi/vmt.c
+++ b/drivers/mtd/ubi/vmt.c
@@ -464,7 +464,7 @@ int ubi_resize_volume(struct ubi_volume_desc *desc, int reserved_pebs)
 		for (i = 0; i < -pebs; i++) {
 			err = ubi_eba_unmap_leb(ubi, vol, reserved_pebs + i);
 			if (err)
-				goto out_acc;
+				goto out_free;
 		}
 		spin_lock(&ubi->volumes_lock);
 		ubi->rsvd_pebs += pebs;
@@ -512,6 +512,8 @@ int ubi_resize_volume(struct ubi_volume_desc *desc, int reserved_pebs)
 		ubi->avail_pebs += pebs;
 		spin_unlock(&ubi->volumes_lock);
 	}
+	return err;
+
 out_free:
 	kfree(new_eba_tbl);
 	return err;
-- 
2.31.1


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

[PATCH 2/2] ubi: Fix unreferenced object reported by kmemleak in ubi_resize_volume()

[Li Zetao]

There is a memory leaks problem reported by kmemleak:

unreferenced object 0xffff888102007a00 (size 128):
  comm "ubirsvol", pid 32090, jiffies 4298464136 (age 2361.231s)
  hex dump (first 32 bytes):
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
  backtrace:
[<ffffffff8176cecd>] __kmalloc+0x4d/0x150
[<ffffffffa02a9a36>] ubi_eba_create_table+0x76/0x170 [ubi]
[<ffffffffa029764e>] ubi_resize_volume+0x1be/0xbc0 [ubi]
[<ffffffffa02a3321>] ubi_cdev_ioctl+0x701/0x1850 [ubi]
[<ffffffff81975d2d>] __x64_sys_ioctl+0x11d/0x170
[<ffffffff83c142a5>] do_syscall_64+0x35/0x80
[<ffffffff83e0006a>] entry_SYSCALL_64_after_hwframe+0x46/0xb0

This is due to a mismatch between create and destroy interfaces, and
in detail that "new_eba_tbl" created by ubi_eba_create_table() but
destroyed by kfree(), while will causing "new_eba_tbl->entries" not
freed.

Fix it by replacing kfree(new_eba_tbl) with
ubi_eba_destroy_table(new_eba_tbl)

Fixes: 799dca34ac54 ("UBI: hide EBA internals")
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
 drivers/mtd/ubi/vmt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/ubi/vmt.c b/drivers/mtd/ubi/vmt.c
index 74637575346e..9fbc64b997ce 100644
--- a/drivers/mtd/ubi/vmt.c
+++ b/drivers/mtd/ubi/vmt.c
@@ -515,7 +515,7 @@ int ubi_resize_volume(struct ubi_volume_desc *desc, int reserved_pebs)
 	return err;
 
 out_free:
-	kfree(new_eba_tbl);
+	ubi_eba_destroy_table(new_eba_tbl);
 	return err;
 }
 
-- 
2.31.1


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

Re: [PATCH 1/2] ubi: Fix use-after-free when volume resizing failed

[Zhihao Cheng]

在 2022/10/21 18:21, Li Zetao 写道:
> There is an use-after-free problem reported by KASAN:
>    ==================================================================
>    BUG: KASAN: use-after-free in ubi_eba_copy_table+0x11f/0x1c0 [ubi]
>    Read of size 8 at addr ffff888101eec008 by task ubirsvol/4735
>
>    CPU: 2 PID: 4735 Comm: ubirsvol
>    Not tainted 6.1.0-rc1-00003-g84fa3304a7fc-dirty #14
>    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
>    BIOS 1.14.0-1.fc33 04/01/2014
>    Call Trace:
>     <TASK>
>     dump_stack_lvl+0x34/0x44
>     print_report+0x171/0x472
>     kasan_report+0xad/0x130
>     ubi_eba_copy_table+0x11f/0x1c0 [ubi]
>     ubi_resize_volume+0x4f9/0xbc0 [ubi]
>     ubi_cdev_ioctl+0x701/0x1850 [ubi]
>     __x64_sys_ioctl+0x11d/0x170
>     do_syscall_64+0x35/0x80
>     entry_SYSCALL_64_after_hwframe+0x46/0xb0
>     </TASK>
>
> When ubi_change_vtbl_record() returns an error in ubi_resize_volume(),
> "new_eba_tbl" will be freed on error handing path, but it is holded
> by "vol->eba_tbl" in ubi_eba_replace_table(). It means that the liftcycle
> of "vol->eba_tbl" and "vol" are different, so when resizing volume in
> next time, it causing an use-after-free fault.
>
> Fix it by not freeing "new_eba_tbl" after it replaced in
> ubi_eba_replace_table(), while will be freed in next volume resizing.
>
> Fixes: 801c135ce73d ("UBI: Unsorted Block Images")
> Signed-off-by: Li Zetao <lizetao1@huawei.com>
> ---
>   drivers/mtd/ubi/vmt.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/ubi/vmt.c b/drivers/mtd/ubi/vmt.c
> index 8fcc0bdf0635..74637575346e 100644
> --- a/drivers/mtd/ubi/vmt.c
> +++ b/drivers/mtd/ubi/vmt.c
> @@ -464,7 +464,7 @@ int ubi_resize_volume(struct ubi_volume_desc *desc, int reserved_pebs)
>   		for (i = 0; i < -pebs; i++) {
>   			err = ubi_eba_unmap_leb(ubi, vol, reserved_pebs + i);
>   			if (err)
> -				goto out_acc;
> +				goto out_free;
>   		}
>   		spin_lock(&ubi->volumes_lock);
>   		ubi->rsvd_pebs += pebs;
> @@ -512,6 +512,8 @@ int ubi_resize_volume(struct ubi_volume_desc *desc, int reserved_pebs)
>   		ubi->avail_pebs += pebs;
>   		spin_unlock(&ubi->volumes_lock);
>   	}
> +	return err;
> +
>   out_free:
>   	kfree(new_eba_tbl);
>   	return err;

Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>


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

Re: [PATCH 2/2] ubi: Fix unreferenced object reported by kmemleak in ubi_resize_volume()

[Zhihao Cheng]

在 2022/10/21 18:21, Li Zetao 写道:
> There is a memory leaks problem reported by kmemleak:
>
> unreferenced object 0xffff888102007a00 (size 128):
>    comm "ubirsvol", pid 32090, jiffies 4298464136 (age 2361.231s)
>    hex dump (first 32 bytes):
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
>    backtrace:
> [<ffffffff8176cecd>] __kmalloc+0x4d/0x150
> [<ffffffffa02a9a36>] ubi_eba_create_table+0x76/0x170 [ubi]
> [<ffffffffa029764e>] ubi_resize_volume+0x1be/0xbc0 [ubi]
> [<ffffffffa02a3321>] ubi_cdev_ioctl+0x701/0x1850 [ubi]
> [<ffffffff81975d2d>] __x64_sys_ioctl+0x11d/0x170
> [<ffffffff83c142a5>] do_syscall_64+0x35/0x80
> [<ffffffff83e0006a>] entry_SYSCALL_64_after_hwframe+0x46/0xb0
>
> This is due to a mismatch between create and destroy interfaces, and
> in detail that "new_eba_tbl" created by ubi_eba_create_table() but
> destroyed by kfree(), while will causing "new_eba_tbl->entries" not
> freed.
>
> Fix it by replacing kfree(new_eba_tbl) with
> ubi_eba_destroy_table(new_eba_tbl)
>
> Fixes: 799dca34ac54 ("UBI: hide EBA internals")
> Signed-off-by: Li Zetao <lizetao1@huawei.com>
> ---
>   drivers/mtd/ubi/vmt.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/ubi/vmt.c b/drivers/mtd/ubi/vmt.c
> index 74637575346e..9fbc64b997ce 100644
> --- a/drivers/mtd/ubi/vmt.c
> +++ b/drivers/mtd/ubi/vmt.c
> @@ -515,7 +515,7 @@ int ubi_resize_volume(struct ubi_volume_desc *desc, int reserved_pebs)
>   	return err;
>   
>   out_free:
> -	kfree(new_eba_tbl);
> +	ubi_eba_destroy_table(new_eba_tbl);
>   	return err;
>   }
>   

Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>


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