[BUG] mei: a possible use-after-free caused by concurrency execution

[BUG] mei: a possible use-after-free caused by concurrency execution

[Tuo Li]

Hello,

Our static analysis tool has identified a potential use-after-free caused
by concurrency execution in drivers/misc/mei/main.c.

Consider the following execution scenario:
(The line numbers can be referred to
https://elixir.bootlin.com/linux/v6.12/source/drivers/misc/mei/main.c)

  mei_release()                            //Line 112
    cl = file->private_data;               //Line 114
    mutex_lock(&dev->device_lock);         //Line 123
    kfree(cl);                             //Line 149
    file->private_data = NULL;             //Line 152
    mutex_unlock(&dev->device_lock);       //Line 154

  mei_read()                               //Line 169
    cl = file->private_data;               //Line 172
    mutex_lock(&dev->device_lock);         //Line 184
    cb = mei_cl_read_cb(cl, file);         //Line 200
    cl_dbg(dev, cl, ...);                  //Line 275
    mutex_unlock(&dev->device_lock);       //Line 276

If mei_release() and mei_read() can execute concurrently and the execution
order is 114, 172, 123, 149 (free), 152, 154, 184, 200 (use), 275 (use),
276, a possible use-after-free can occur.

Our static analysis tool reports this use-after-free when analyzing Linux
6.12. The tool deduces lock() and unlock() pairs with alias analysis. It
then applies data flow analysis to detect use-after-free across
synchronization points.

I am not quite sure whether this possible use-after-free is real and how to
fix it if it is real.
Any feedback would be appreciated. Thanks!

Sincerely,
Tuo Li

Re: [BUG] mei: a possible use-after-free caused by concurrency execution

[Greg KH]

On Thu, Jan 16, 2025 at 05:26:33PM +0800, Tuo Li wrote:
> Hello,
> 
> Our static analysis tool has identified a potential use-after-free caused
> by concurrency execution in drivers/misc/mei/main.c.
> 
> Consider the following execution scenario:
> (The line numbers can be referred to
> https://elixir.bootlin.com/linux/v6.12/source/drivers/misc/mei/main.c)
> 
>   mei_release()                            //Line 112
>     cl = file->private_data;               //Line 114
>     mutex_lock(&dev->device_lock);         //Line 123
>     kfree(cl);                             //Line 149
>     file->private_data = NULL;             //Line 152
>     mutex_unlock(&dev->device_lock);       //Line 154
> 
>   mei_read()                               //Line 169
>     cl = file->private_data;               //Line 172
>     mutex_lock(&dev->device_lock);         //Line 184
>     cb = mei_cl_read_cb(cl, file);         //Line 200
>     cl_dbg(dev, cl, ...);                  //Line 275
>     mutex_unlock(&dev->device_lock);       //Line 276
> 
> If mei_release() and mei_read() can execute concurrently and the execution
> order is 114, 172, 123, 149 (free), 152, 154, 184, 200 (use), 275 (use),
> 276, a possible use-after-free can occur.

How can release run at the same time read happens?  release only happens
after all references are dropped, right?

> Our static analysis tool reports this use-after-free when analyzing Linux
> 6.12. The tool deduces lock() and unlock() pairs with alias analysis. It
> then applies data flow analysis to detect use-after-free across
> synchronization points.
> 
> I am not quite sure whether this possible use-after-free is real and how to
> fix it if it is real.

Test it and see!  And if you feel a fix is needed, please provide a
patch.

thanks,

greg k-h

Re: [BUG] mei: a possible use-after-free caused by concurrency execution

[Tuo Li]

On 2025/1/16 17:51, Greg KH wrote:
> On Thu, Jan 16, 2025 at 05:26:33PM +0800, Tuo Li wrote:
>> Hello,
>>
>> Our static analysis tool has identified a potential use-after-free caused
>> by concurrency execution in drivers/misc/mei/main.c.
>>
>> Consider the following execution scenario:
>> (The line numbers can be referred to
>> https://elixir.bootlin.com/linux/v6.12/source/drivers/misc/mei/main.c)
>>
>>   mei_release()                            //Line 112
>>     cl = file->private_data;               //Line 114
>>     mutex_lock(&dev->device_lock);         //Line 123
>>     kfree(cl);                             //Line 149
>>     file->private_data = NULL;             //Line 152
>>     mutex_unlock(&dev->device_lock);       //Line 154
>>
>>   mei_read()                               //Line 169
>>     cl = file->private_data;               //Line 172
>>     mutex_lock(&dev->device_lock);         //Line 184
>>     cb = mei_cl_read_cb(cl, file);         //Line 200
>>     cl_dbg(dev, cl, ...);                  //Line 275
>>     mutex_unlock(&dev->device_lock);       //Line 276
>>
>> If mei_release() and mei_read() can execute concurrently and the execution
>> order is 114, 172, 123, 149 (free), 152, 154, 184, 200 (use), 275 (use),
>> 276, a possible use-after-free can occur.
> 
> How can release run at the same time read happens?  release only happens
> after all references are dropped, right?
> 
>> Our static analysis tool reports this use-after-free when analyzing Linux
>> 6.12. The tool deduces lock() and unlock() pairs with alias analysis. It
>> then applies data flow analysis to detect use-after-free across
>> synchronization points.
>>
>> I am not quite sure whether this possible use-after-free is real and how to
>> fix it if it is real.
> 
> Test it and see!  And if you feel a fix is needed, please provide a
> patch.
> 
> thanks,
> 
> greg k-h

Hello,

I really appreciate the feedback! I apologize for the inconvenience this
report has caused.

Sincerely,
Tuo Li