RE: [PATCH 01/29] scsi: aacraid: Fix udev inquiry race condition

2017-12-26 Thread Raghava Aditya Renukunta 


> -Original Message-
> From: Bart Van Assche [mailto:bart.vanass...@wdc.com]
> Sent: Thursday, December 21, 2017 9:54 AM
> To: j...@linux.vnet.ibm.com; Raghava Aditya Renukunta
> ; linux-scsi@vger.kernel.org;
> martin.peter...@oracle.com
> Cc: dl-esc-Aacraid Linux Driver ;
> gpicc...@linux.vnet.ibm.com; Tom White ;
> Scott Benesh 
> Subject: Re: [PATCH 01/29] scsi: aacraid: Fix udev inquiry race condition
> 
> EXTERNAL EMAIL
> 
> 
> On Thu, 2017-12-21 at 09:33 -0800, Raghava Aditya Renukunta wrote:
> > + char *cp;
> > + char *cname = kmalloc(sizeof(sup_adap_info-
> >adapter_type_text),
> > + GFP_ATOMIC);
> 
> Why did you choose to use GFP_ATOMIC instead of GFP_KERNEL in the
> above
> kmalloc() call?

It was mainly because of a trace call that cut thru the kmalloc call. 
 
> > +
> > + if (!cname)
> > + return;
> > +
> > + cp = cname;
> > + memcpy(cname, sup_adap_info->adapter_type_text,
> > + sizeof(sup_adap_info->adapter_type_text));
> 
> Is the sup_adap_info->adapter_type_text a string that is \0-terminated? If
> so,
> have you considered to use kmemdup() instead of kmalloc() + memcpy()?

I will take that into consideration Bart.

Thank you 
Raghava Aditya

> Thanks,
> 
> Bart.

Re: [PATCH 01/29] scsi: aacraid: Fix udev inquiry race condition

2017-12-21 Thread Bart Van Assche 
On Thu, 2017-12-21 at 09:33 -0800, Raghava Aditya Renukunta wrote:
> + char *cp;
> + char *cname = kmalloc(sizeof(sup_adap_info->adapter_type_text),
> + GFP_ATOMIC);

Why did you choose to use GFP_ATOMIC instead of GFP_KERNEL in the above
kmalloc() call?

> +
> + if (!cname)
> + return;
> +
> + cp = cname;
> + memcpy(cname, sup_adap_info->adapter_type_text,
> + sizeof(sup_adap_info->adapter_type_text));

Is the sup_adap_info->adapter_type_text a string that is \0-terminated? If so,
have you considered to use kmemdup() instead of kmalloc() + memcpy()?

Thanks,

Bart.

[PATCH 01/29] scsi: aacraid: Fix udev inquiry race condition

2017-12-21 Thread Raghava Aditya Renukunta 
When udev requests for a devices inquiry string, it might create multiple
threads causing a race condition on the shared inquiry resource string.

Created a buffer with the string for each thread.

Cc: 
Fixes: 3bc8070fb75b3315 ([SCSI] aacraid: SMC vendor identification)
Signed-off-by: Raghava Aditya Renukunta 
---
 drivers/scsi/aacraid/aachba.c | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index af3e4d3..f264515 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -913,8 +913,18 @@ static void setinqstr(struct aac_dev *dev, void *data, int 
tindex)
memset(str, ' ', sizeof(*str));
 
if (sup_adap_info->adapter_type_text[0]) {
-   char *cp = sup_adap_info->adapter_type_text;
int c;
+   char *cp;
+   char *cname = kmalloc(sizeof(sup_adap_info->adapter_type_text),
+   GFP_ATOMIC);
+
+   if (!cname)
+   return;
+
+   cp = cname;
+   memcpy(cname, sup_adap_info->adapter_type_text,
+   sizeof(sup_adap_info->adapter_type_text));
+
if ((cp[0] == 'A') && (cp[1] == 'O') && (cp[2] == 'C'))
inqstrcpy("SMC", str->vid);
else {
@@ -923,7 +933,7 @@ static void setinqstr(struct aac_dev *dev, void *data, int 
tindex)
++cp;
c = *cp;
*cp = '\0';
-   inqstrcpy(sup_adap_info->adapter_type_text, str->vid);
+   inqstrcpy(cname, str->vid);
*cp = c;
while (*cp && *cp != ' ')
++cp;
@@ -937,8 +947,8 @@ static void setinqstr(struct aac_dev *dev, void *data, int 
tindex)
cp[sizeof(str->pid)] = '\0';
}
inqstrcpy (cp, str->pid);
-   if (c)
-   cp[sizeof(str->pid)] = c;
+
+   kfree(cname);
} else {
struct aac_driver_ident *mp = 
aac_get_driver_ident(dev->cardtype);
 
-- 
2.9.4