Re: [PATCH RFC] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier

[ZhaoLong Wang <wangzhaolong1@huawei.com>]

>>
>>> 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);