Bug report: usb: cdns3: cdns3-gadget.c: use-after-free bug in cdns3_gadget_exit due to race condition

Bug report: usb: cdns3: cdns3-gadget.c: use-after-free bug in cdns3_gadget_exit due to race condition

[sicong]

usb: cdns3: cdns3-gadget.c: use-after-free bug in cdns3_gadget_exit due to      
race condition                                                                  
                                                                                
This bug exists in drivers/usb/cdns3/cdns3-gadget.c. Function                   
__cdns3_gadget_init will call cdns3_gadget_start to do the futher jobs          
during the initialization proccess of cdns3 gadget. In cdns3_gadget_start,      
&priv_dev->pending_status_wq is bound with cdns3_pending_setup_status_handler.  
Then this work will be added to system_freezable_wq in cdns3_gadget_ep0_queue.  
Here is the code.                                                               
queue_work(system_freezable_wq, &priv_dev->pending_status_wq);                  
                                                                                
If we call cdns3_gadget_exit to remove the device and make cleanup,             
there are some unfinished works. This function will call cdns3_free_all_eps to  
free all the endpoints. However, if cdns3_pending_setup_status_handler is       
scheduled to run after the free job, it will cause use-after-free error as      
cdns3_pending_setup_status_handler will use the endpoint in the following code. 
request->complete(&priv_dev->eps[0]->endpoint, request);                        
                                                                                
The possible execution flow that may lead to this issue is as follows:          
CPU0                    CPU1                                                    
                      | __cdns3_gadget_init                                     
                      | cdns3_gadget_start                                      
cdns3_gadget_exit     |                                                             
cdns3_free_all_eps    |                                                             
devm_kfree (free)     |                                                             
                      | cdns3_pending_setup_status_handler                       
                      | &priv_dev->eps[0]->endpoint (use)    

                                                                                
This bug may be fixed by adding the following code in cdns3_gadget_exit.        
cancel_work_sync(&priv_dev->pending_status_wq);                                 
cancel_work_sync(&priv_dev->aligned_buf_wq);                                    
                                                                                
Best regards,                                                                   
Sicong Huang     

Re: Bug report: usb: cdns3: cdns3-gadget.c: use-after-free bug in cdns3_gadget_exit due to race condition

[Greg KH]

On Sat, May 11, 2024 at 08:49:04PM +0800, sicong wrote:
> 
> 
> usb: cdns3: cdns3-gadget.c: use-after-free bug in cdns3_gadget_exit due to      
> race condition                                                                  
>                                                                                 
> This bug exists in drivers/usb/cdns3/cdns3-gadget.c. Function                   
> __cdns3_gadget_init will call cdns3_gadget_start to do the futher jobs          
> during the initialization proccess of cdns3 gadget. In cdns3_gadget_start,      
> &priv_dev->pending_status_wq is bound with cdns3_pending_setup_status_handler.  
> Then this work will be added to system_freezable_wq in cdns3_gadget_ep0_queue.  
> Here is the code.                                                               
> queue_work(system_freezable_wq, &priv_dev->pending_status_wq);                  
>                                                                                 
> If we call cdns3_gadget_exit to remove the device and make cleanup,             
> there are some unfinished works. This function will call cdns3_free_all_eps to  
> free all the endpoints. However, if cdns3_pending_setup_status_handler is       
> scheduled to run after the free job, it will cause use-after-free error as      
> cdns3_pending_setup_status_handler will use the endpoint in the following code. 
> request->complete(&priv_dev->eps[0]->endpoint, request);                        
>                                                                                 
> The possible execution flow that may lead to this issue is as follows:          
> CPU0                    CPU1                                                    
>                       | __cdns3_gadget_init                                     
>                       | cdns3_gadget_start                                      
> cdns3_gadget_exit     |                                                             
> cdns3_free_all_eps    |                                                             
> devm_kfree (free)     |                                                             
>                       | cdns3_pending_setup_status_handler                       
>                       | &priv_dev->eps[0]->endpoint (use)    
> 
>                                                                                 
> This bug may be fixed by adding the following code in cdns3_gadget_exit.        
> cancel_work_sync(&priv_dev->pending_status_wq);                                 
> cancel_work_sync(&priv_dev->aligned_buf_wq);                                    

Can you please provide a patch for this so we can apply it and give you
the credit for fixing the issue?

thanks,

greg k-h