| Message ID | 20171221173420.8213-2-RaghavaAditya.Renukunta@microsemi.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
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.
> -----Original Message----- > From: Bart Van Assche [mailto:Bart.VanAssche@wdc.com] > Sent: Thursday, December 21, 2017 9:54 AM > To: jejb@linux.vnet.ibm.com; Raghava Aditya Renukunta > <RaghavaAditya.Renukunta@microsemi.com>; linux-scsi@vger.kernel.org; > martin.petersen@oracle.com > Cc: dl-esc-Aacraid Linux Driver <aacraid@microsemi.com>; > gpiccoli@linux.vnet.ibm.com; Tom White <tom.white@microsemi.com>; > Scott Benesh <scott.benesh@microsemi.com> > 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.
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);
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: <stable@vger.kernel.org> Fixes: 3bc8070fb75b3315 ([SCSI] aacraid: SMC vendor identification) Signed-off-by: Raghava Aditya Renukunta <RaghavaAditya.Renukunta@microsemi.com> --- drivers/scsi/aacraid/aachba.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)