Re: [PATCH v2 21/30] cxlflash: Fix to prevent workq from accessing freed memory

2015-09-21 Thread Matthew R. Ochs 
> On Sep 21, 2015, at 7:25 AM, Tomas Henzl  wrote:
> On 16.9.2015 23:31, Matthew R. Ochs wrote:
>> The workq can process work in parallel with a remove event, leading
>> to a condition where the workq handler can access freed memory.
>> 
>> To remedy, the workq should be terminated prior to freeing memory. Move
>> the termination call earlier in remove and use cancel_work_sync() instead
>> of flush_work() as there is not a need to process any scheduled work when
>> shutting down.
>> 
>> Signed-off-by: Matthew R. Ochs 
>> Signed-off-by: Manoj N. Kumar 
>> ---
>> drivers/scsi/cxlflash/main.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
>> index 1856a73..1625aea 100644
>> --- a/drivers/scsi/cxlflash/main.c
>> +++ b/drivers/scsi/cxlflash/main.c
>> @@ -736,12 +736,12 @@ static void cxlflash_remove(struct pci_dev *pdev)
>>  scsi_remove_host(cfg->host);
>>  /* Fall through */
>>  case INIT_STATE_AFU:
>> +cancel_work_sync(>work_q);
>>  term_afu(cfg);
> 
> You disable irqs after a call to cancel_work_sync.
> That means a late int could trigger the workqueue again?
> Please disable irqs earlier - as described in Documentation/PCI/pci.txt

I'll change the order here such that the work is cancelled after
term_afu() is called.

-matt

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v2 21/30] cxlflash: Fix to prevent workq from accessing freed memory

2015-09-21 Thread Tomas Henzl 
On 16.9.2015 23:31, Matthew R. Ochs wrote:
> The workq can process work in parallel with a remove event, leading
> to a condition where the workq handler can access freed memory.
>
> To remedy, the workq should be terminated prior to freeing memory. Move
> the termination call earlier in remove and use cancel_work_sync() instead
> of flush_work() as there is not a need to process any scheduled work when
> shutting down.
>
> Signed-off-by: Matthew R. Ochs 
> Signed-off-by: Manoj N. Kumar 
> ---
>  drivers/scsi/cxlflash/main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
> index 1856a73..1625aea 100644
> --- a/drivers/scsi/cxlflash/main.c
> +++ b/drivers/scsi/cxlflash/main.c
> @@ -736,12 +736,12 @@ static void cxlflash_remove(struct pci_dev *pdev)
>   scsi_remove_host(cfg->host);
>   /* Fall through */
>   case INIT_STATE_AFU:
> + cancel_work_sync(>work_q);
>   term_afu(cfg);

You disable irqs after a call to cancel_work_sync.
That means a late int could trigger the workqueue again?
Please disable irqs earlier - as described in Documentation/PCI/pci.txt

>   case INIT_STATE_PCI:
>   pci_release_regions(cfg->dev);
>   pci_disable_device(pdev);
>   case INIT_STATE_NONE:
> - flush_work(>work_q);
>   free_mem(cfg);
>   scsi_host_put(cfg->host);
>   break;

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 21/30] cxlflash: Fix to prevent workq from accessing freed memory

2015-09-16 Thread Matthew R. Ochs 
The workq can process work in parallel with a remove event, leading
to a condition where the workq handler can access freed memory.

To remedy, the workq should be terminated prior to freeing memory. Move
the termination call earlier in remove and use cancel_work_sync() instead
of flush_work() as there is not a need to process any scheduled work when
shutting down.

Signed-off-by: Matthew R. Ochs 
Signed-off-by: Manoj N. Kumar 
---
 drivers/scsi/cxlflash/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 1856a73..1625aea 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -736,12 +736,12 @@ static void cxlflash_remove(struct pci_dev *pdev)
scsi_remove_host(cfg->host);
/* Fall through */
case INIT_STATE_AFU:
+   cancel_work_sync(>work_q);
term_afu(cfg);
case INIT_STATE_PCI:
pci_release_regions(cfg->dev);
pci_disable_device(pdev);
case INIT_STATE_NONE:
-   flush_work(>work_q);
free_mem(cfg);
scsi_host_put(cfg->host);
break;
-- 
2.1.0

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 21/30] cxlflash: Fix to prevent workq from accessing freed memory

2015-09-16 Thread Matthew R. Ochs 
The workq can process work in parallel with a remove event, leading
to a condition where the workq handler can access freed memory.

To remedy, the workq should be terminated prior to freeing memory. Move
the termination call earlier in remove and use cancel_work_sync() instead
of flush_work() as there is not a need to process any scheduled work when
shutting down.

Signed-off-by: Matthew R. Ochs 
Signed-off-by: Manoj N. Kumar 
---
 drivers/scsi/cxlflash/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 1856a73..1625aea 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -736,12 +736,12 @@ static void cxlflash_remove(struct pci_dev *pdev)
scsi_remove_host(cfg->host);
/* Fall through */
case INIT_STATE_AFU:
+   cancel_work_sync(>work_q);
term_afu(cfg);
case INIT_STATE_PCI:
pci_release_regions(cfg->dev);
pci_disable_device(pdev);
case INIT_STATE_NONE:
-   flush_work(>work_q);
free_mem(cfg);
scsi_host_put(cfg->host);
break;
-- 
2.1.0

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev