Messages in this thread | | | Date | Thu, 12 Oct 2023 17:31:11 +0800 | Subject | Re: [PATCH RFC] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier | From | ZhaoLong Wang <> |
| |
>> >>> 3. P1 P2 >>> gluebi_create -> mtd_device_register -> add_mtd_device: >>> device_register // dev/mtd1 is visible >>> >>> fd = open(/dev/mtd1, O_WRONLY) >>> gluebi_get_device >>> gluebi->desc = ubi_open_volume >>> >>> ftl_add_mtd >>> mtd_read >>> gluebi_read >>> gluebi->desc is not ERR_PTR/NULL >>> >>> close(fd) >>> gluebi_put_device >>> ubi_close_volume >>> kfree(desc) >>> ubi_read(gluebi->desc) // UAF (×) >>> >> >> Yes, it's also a problem. Perhaps it should be set to NULL after >> destroying gluebi->desc. > > The key point is that 'gluebi->desc' check & usage is not atomic in > gluebi_read. So following patch still can't handle situation 3. >
Setting the desc to NULL works because mutex_lock "mtd_table_mutex" is held on all paths where ftl_add_mtd() is called.
ubi_gluebi_init ubi_register_volume_notifier ubi_enumerate_volumes ubi_notify_all gluebi_notify nb->notifier_call() gluebi_create mtd_device_register mtd_device_parse_register add_mtd_device mutex_lock(&mtd_table_mutex); blktrans_notify_add not->add() ftl_add_mtd tr->add_mtd() scan_header mtd_read mtd_read mtd_read_oob gluebi_read mtd->read() gluebi->desc - NULL mutex_unlock(&mtd_table_mutex);
ubi_cdev_ioctl ubi_create_volume ubi_volume_notify blocking_notifier_call_chain [kernel/notifier.c] notifier_call_chain gluebi_notify nb->notifier_call() gluebi_create mtd_device_register mtd_device_parse_register add_mtd_device mutex_lock(&mtd_table_mutex); blktrans_notify_add not->add() ftl_add_mtd tr->add_mtd() scan_header mtd_read(part->mbd.mtd, mtd_read_oob gluebi_read mtd->read() ubi_read(gluebi->desc mutex_unlock(&mtd_table_mutex);
load_module ftl register_mtd_blktrans mutex_lock(&mtd_table_mutex); ftl_add_mtd not->add() scan_header mtd_read(part->mbd.mtd, mtd_read_oob gluebi_read mtd->read() ubi_read(gluebi->desc mutex_unlock(&mtd_table_mutex);
This lock is also held during the process of closing the file.
close(fd) mtdchar_close put_mtd_device mutex_lock(&mtd_table_mutex); __put_mtd_device gluebi_put_device ubi_close_volume kfree(desc) // desc == NULL mutex_unlock(&mtd_table_mutex);
| |