PCI: endpoint: use after free in pci_epf_unregister_driver()

Message ID 20180531062148.qnhcnnibz2ql6soa@kili.mountain
State Accepted
Delegated to: Lorenzo Pieralisi
Headers show
Series
  • PCI: endpoint: use after free in pci_epf_unregister_driver()
Related show

Commit Message

Dan Carpenter May 31, 2018, 6:21 a.m.
We need to use list_for_each_entry_safe() because the
pci_ep_cfs_remove_epf_group() function frees "group".

Fixes: ef1433f717a2 ("PCI: endpoint: Create configfs entry for each pci_epf_device_id table entry")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Comments

Lorenzo Pieralisi June 29, 2018, 10 a.m. | #1
On Thu, May 31, 2018 at 09:21:48AM +0300, Dan Carpenter wrote:
> We need to use list_for_each_entry_safe() because the
> pci_ep_cfs_remove_epf_group() function frees "group".
> 
> Fixes: ef1433f717a2 ("PCI: endpoint: Create configfs entry for each pci_epf_device_id table entry")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
> index 523a8cab3bfb..bf53fad636a5 100644
> --- a/drivers/pci/endpoint/pci-epf-core.c
> +++ b/drivers/pci/endpoint/pci-epf-core.c
> @@ -145,10 +145,10 @@ EXPORT_SYMBOL_GPL(pci_epf_alloc_space);
>   */
>  void pci_epf_unregister_driver(struct pci_epf_driver *driver)
>  {
> -	struct config_group *group;
> +	struct config_group *group, *tmp;
>  
>  	mutex_lock(&pci_epf_mutex);
> -	list_for_each_entry(group, &driver->epf_group, group_entry)
> +	list_for_each_entry_safe(group, tmp, &driver->epf_group, group_entry)
>  		pci_ep_cfs_remove_epf_group(group);
>  	list_del(&driver->epf_group);
>  	mutex_unlock(&pci_epf_mutex);

Kishon, I need your ACK to merge this fix, thanks.

Lorenzo
Kishon Vijay Abraham I June 29, 2018, 10 a.m. | #2
On Friday 29 June 2018 03:30 PM, Lorenzo Pieralisi wrote:
> On Thu, May 31, 2018 at 09:21:48AM +0300, Dan Carpenter wrote:
>> We need to use list_for_each_entry_safe() because the
>> pci_ep_cfs_remove_epf_group() function frees "group".
>>
>> Fixes: ef1433f717a2 ("PCI: endpoint: Create configfs entry for each pci_epf_device_id table entry")
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>
>> diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
>> index 523a8cab3bfb..bf53fad636a5 100644
>> --- a/drivers/pci/endpoint/pci-epf-core.c
>> +++ b/drivers/pci/endpoint/pci-epf-core.c
>> @@ -145,10 +145,10 @@ EXPORT_SYMBOL_GPL(pci_epf_alloc_space);
>>   */
>>  void pci_epf_unregister_driver(struct pci_epf_driver *driver)
>>  {
>> -	struct config_group *group;
>> +	struct config_group *group, *tmp;
>>  
>>  	mutex_lock(&pci_epf_mutex);
>> -	list_for_each_entry(group, &driver->epf_group, group_entry)
>> +	list_for_each_entry_safe(group, tmp, &driver->epf_group, group_entry)
>>  		pci_ep_cfs_remove_epf_group(group);
>>  	list_del(&driver->epf_group);
>>  	mutex_unlock(&pci_epf_mutex);
> 
> Kishon, I need your ACK to merge this fix, thanks.

Looks correct to me.
Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
> 
> Lorenzo
>
Lorenzo Pieralisi June 29, 2018, 1:47 p.m. | #3
On Thu, May 31, 2018 at 09:21:48AM +0300, Dan Carpenter wrote:
> We need to use list_for_each_entry_safe() because the
> pci_ep_cfs_remove_epf_group() function frees "group".
> 
> Fixes: ef1433f717a2 ("PCI: endpoint: Create configfs entry for each pci_epf_device_id table entry")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c

Applied to pci/controller-fixes to be tentatively merged for -rc4,
thanks.

Lorenzo

> index 523a8cab3bfb..bf53fad636a5 100644
> --- a/drivers/pci/endpoint/pci-epf-core.c
> +++ b/drivers/pci/endpoint/pci-epf-core.c
> @@ -145,10 +145,10 @@ EXPORT_SYMBOL_GPL(pci_epf_alloc_space);
>   */
>  void pci_epf_unregister_driver(struct pci_epf_driver *driver)
>  {
> -	struct config_group *group;
> +	struct config_group *group, *tmp;
>  
>  	mutex_lock(&pci_epf_mutex);
> -	list_for_each_entry(group, &driver->epf_group, group_entry)
> +	list_for_each_entry_safe(group, tmp, &driver->epf_group, group_entry)
>  		pci_ep_cfs_remove_epf_group(group);
>  	list_del(&driver->epf_group);
>  	mutex_unlock(&pci_epf_mutex);

Patch

diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
index 523a8cab3bfb..bf53fad636a5 100644
--- a/drivers/pci/endpoint/pci-epf-core.c
+++ b/drivers/pci/endpoint/pci-epf-core.c
@@ -145,10 +145,10 @@  EXPORT_SYMBOL_GPL(pci_epf_alloc_space);
  */
 void pci_epf_unregister_driver(struct pci_epf_driver *driver)
 {
-	struct config_group *group;
+	struct config_group *group, *tmp;
 
 	mutex_lock(&pci_epf_mutex);
-	list_for_each_entry(group, &driver->epf_group, group_entry)
+	list_for_each_entry_safe(group, tmp, &driver->epf_group, group_entry)
 		pci_ep_cfs_remove_epf_group(group);
 	list_del(&driver->epf_group);
 	mutex_unlock(&pci_epf_mutex);