Re: [PATCH] megaraid: Fix a sleep-in-atomic bug

[Jia-Ju Bai]

On 05/31/2017 06:18 PM, Sumit Saxena wrote:

-Original Message-
From: Jia-Ju Bai [mailto:baijiaju1...@163.com]
Sent: Wednesday, May 31, 2017 8:27 AM
To: kashyap.de...@broadcom.com; sumit.sax...@broadcom.com;
shivasharan.srikanteshw...@broadcom.com; j...@linux.vnet.ibm.com;
martin.peter...@oracle.com
Cc: megaraidlinux@broadcom.com; linux-scsi@vger.kernel.org; linux-
ker...@vger.kernel.org; Jia-Ju Bai
Subject: [PATCH] megaraid: Fix a sleep-in-atomic bug

The driver may sleep under a spin lock, and the function call path is:
mraid_mm_attach_buf (acquire the lock by spin_lock_irqsave)
  pci_pool_alloc(GFP_KERNEL) -->  may sleep

To fix it, the "GFP_KERNEL" is replaced with "GFP_ATOMIC".

Signed-off-by: Jia-Ju Bai
---
drivers/scsi/megaraid/megaraid_mm.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/megaraid/megaraid_mm.c
b/drivers/scsi/megaraid/megaraid_mm.c
index 4cf9ed9..c43afb8 100644
--- a/drivers/scsi/megaraid/megaraid_mm.c
+++ b/drivers/scsi/megaraid/megaraid_mm.c
@@ -574,7 +574,7 @@

kioc->pool_index = right_pool;
kioc->free_buf   = 1;
-   kioc->buf_vaddr  = pci_pool_alloc(pool->handle, GFP_KERNEL,
+   kioc->buf_vaddr  = pci_pool_alloc(pool->handle, GFP_ATOMIC,
>buf_paddr);
spin_unlock_irqrestore(>lock, flags);

This is very old driver and reached EOL. Did you face any issue because of
this bug or discover this through code review?
Anyways patch looks good to me.

Acked-by: Sumit Saxena


--
1.7.9.5


Hi,

This bug is found by a static analysis tool and my code review.

Jia-Ju Bai

RE: [PATCH] megaraid: Fix a sleep-in-atomic bug

[Sumit Saxena]

>-Original Message-
>From: Jia-Ju Bai [mailto:baijiaju1...@163.com]
>Sent: Wednesday, May 31, 2017 8:27 AM
>To: kashyap.de...@broadcom.com; sumit.sax...@broadcom.com;
>shivasharan.srikanteshw...@broadcom.com; j...@linux.vnet.ibm.com;
>martin.peter...@oracle.com
>Cc: megaraidlinux@broadcom.com; linux-scsi@vger.kernel.org; linux-
>ker...@vger.kernel.org; Jia-Ju Bai
>Subject: [PATCH] megaraid: Fix a sleep-in-atomic bug
>
>The driver may sleep under a spin lock, and the function call path is:
>mraid_mm_attach_buf (acquire the lock by spin_lock_irqsave)
>  pci_pool_alloc(GFP_KERNEL) --> may sleep
>
>To fix it, the "GFP_KERNEL" is replaced with "GFP_ATOMIC".
>
>Signed-off-by: Jia-Ju Bai 
>---
> drivers/scsi/megaraid/megaraid_mm.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/scsi/megaraid/megaraid_mm.c
>b/drivers/scsi/megaraid/megaraid_mm.c
>index 4cf9ed9..c43afb8 100644
>--- a/drivers/scsi/megaraid/megaraid_mm.c
>+++ b/drivers/scsi/megaraid/megaraid_mm.c
>@@ -574,7 +574,7 @@
>
>   kioc->pool_index= right_pool;
>   kioc->free_buf  = 1;
>-  kioc->buf_vaddr = pci_pool_alloc(pool->handle, GFP_KERNEL,
>+  kioc->buf_vaddr = pci_pool_alloc(pool->handle, GFP_ATOMIC,
>   >buf_paddr);
>   spin_unlock_irqrestore(>lock, flags);

This is very old driver and reached EOL. Did you face any issue because of
this bug or discover this through code review?
Anyways patch looks good to me.

Acked-by: Sumit Saxena 

>
>--
>1.7.9.5
>