Re: [PATCH] fix kzalloc in scsi device handler

[Rolf Eike Beer <eike-kernel@sf-tec.de>]

[-- Attachment #1: Type: Text/Plain, Size: 2484 bytes --]

Moger, Babu wrote:
> > -----Original Message-----
> > From: Rolf Eike Beer [mailto:eike-kernel@sf-tec.de]
> > Sent: Tuesday, November 16, 2010 11:36 AM
> > To: Moger, Babu
> > Cc: Hillf Danton; linux-scsi@vger.kernel.org; Chandra Seetharaman
> > Subject: Re: [PATCH] fix kzalloc in scsi device handler
> > 
> > Moger, Babu wrote:
> > > Agree.. But this may not fix the problem completely.. Please see my
> > > comments below.. Also copied Chandra if he has any comments..
> > > 
> > > > -----Original Message-----
> > > > From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
> > > > owner@vger.kernel.org] On Behalf Of Hillf Danton
> > > > Sent: Tuesday, November 16, 2010 7:34 AM
> > > > To: linux-scsi@vger.kernel.org
> > > > Subject: [PATCH] fix kzalloc in scsi device handler
> > > > 
> > > > They look like typo, since there are four instances of kzalloc
> > 
> > almost
> > 
> > > > typed by the same finger.
> > > > 
> > > > The pointer is replaced with a struct, which helps kzalloc return
> > > > correct result.
> > > > 
> > > > thanks for any comment on the possibility that mem overflow could
> > > > happen.
> > > > 
> > > > Signed-off-by: Hillf Danton <dhillf@gmail.com>
> > > > ---
> > > > 
> > > > --- a/drivers/scsi/device_handler/scsi_dh_alua.c	2010-11-01
> > > > 19:54:12.000000000 +0800
> > > > +++ b/drivers/scsi/device_handler/scsi_dh_alua.c	2010-11-16
> > > > 20:40:36.000000000 +0800
> > > > @@ -759,7 +759,7 @@ static int alua_bus_attach(struct scsi_d
> > > > 
> > > >  	unsigned long flags;
> > > >  	int err = SCSI_DH_OK;
> > > > 
> > > > -	scsi_dh_data = kzalloc(sizeof(struct scsi_device_handler *)
> > > > +	scsi_dh_data = kzalloc(sizeof(struct scsi_dh_data)
> > > > 
> > > >  			       + sizeof(*h) , GFP_KERNEL);
> > > 
> > > I think this should be like this below.
> > > 
> > >       scsi_dh_data = kzalloc(sizeof(struct scsi_dh_data)
> > >       
> > >   			       + sizeof(struct alua_dh_data) , GFP_KERNEL);
> > 
> > How about kzalloc(sizeof(*scsi_dh_data) + sizeof(*h), GFP_KERNEL)?
> 
> No.. This would only allocate couple of pointers..  The below should be
> good enough..

No, it would not.

>      scsi_dh_data = kzalloc(sizeof(struct scsi_dh_data)
>    			       + sizeof(struct alua_dh_data) , GFP_KERNEL);

And that is exactly what I tried to avoid. Look at the * before the variable 
name. That would allocate the space for what that pointer is pointing to.

Eike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]