lkml.org 
[lkml]   [2008]   [Mar]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH resend] isd200: Allocate sense_buffer for hacked up scsi_cmnd
    From
    Date
    On Wed, 2008-03-12 at 17:24 +0200, Boaz Harrosh wrote:
    > On Wed, Mar 12 2008 at 17:10 +0200, Alan Stern <stern@rowland.harvard.edu> wrote:
    > > On Wed, 12 Mar 2008, Boaz Harrosh wrote:
    > >
    > >> Since the separation of sense_buffer from scsi_cmnd, Drivers that hack their
    > >> own struct scsi_cmnd like here isd200, must also take care of their own
    > >> sense_buffer.
    > >
    > > Did you run this through checkpatch?
    > >
    > > Acked-by: Alan Stern <stern@rowland.harvard.edu>
    > >
    > > Alan Stern
    > >
    > > --
    > No I did not, Thanks. Here it is again.
    > Again this is for 2.6.25 rc-fixes a NULL dereference bugfix!
    >
    > ---
    > From: Boaz Harrosh <bharrosh@panasas.com>
    > Date: Tue, 11 Mar 2008 17:23:06 +0200
    > Subject: [PATCH] isd200: Allocate sense_buffer for hacked up scsi_cmnd
    >
    > Since the separation of sense_buffer from scsi_cmnd, Drivers that hack their
    > own struct scsi_cmnd like here isd200, must also take care of their own
    > sense_buffer.
    >
    > Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
    > ---
    > drivers/usb/storage/isd200.c | 8 ++++++--
    > 1 files changed, 6 insertions(+), 2 deletions(-)
    >
    > diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
    > index 2ae1e86..ac1764b 100644
    > --- a/drivers/usb/storage/isd200.c
    > +++ b/drivers/usb/storage/isd200.c
    > @@ -294,6 +294,7 @@ struct isd200_info {
    > unsigned char MaxLUNs;
    > struct scsi_cmnd srb;
    > struct scatterlist sg;
    > + u8 *sense_buffer;

    There's no real need to add this parameter, since all you're doing is
    assigning it to srb.sense_buffer, there's no need to have an extra
    pointer for it.

    > };
    >
    >
    > @@ -1469,6 +1470,7 @@ static void isd200_free_info_ptrs(void *info_)
    > if (info) {
    > kfree(info->id);
    > kfree(info->RegsBuf);
    > + kfree(info->sense_buffer);
    > }
    > }
    >
    > @@ -1494,11 +1496,13 @@ static int isd200_init_info(struct us_data *us)
    > kzalloc(sizeof(struct hd_driveid), GFP_KERNEL);
    > info->RegsBuf = (unsigned char *)
    > kmalloc(sizeof(info->ATARegs), GFP_KERNEL);
    > - if (!info->id || !info->RegsBuf) {
    > + info->sense_buffer = kmalloc(SCSI_SENSE_BUFFERSIZE, GFP_KERNEL);
    ^ kzalloc is probably best
    > + if (!info->id || !info->RegsBuf || !info->sense_buffer) {
    > isd200_free_info_ptrs(info);
    > kfree(info);

    Needs to be a kfree(info->sense_buffer) to avoid leaks if the others
    couldn't allocate ... it also looks like there are missing
    kfree(info->id) and (info->RegsBuf) that fix leaks that aren't part of
    this patch.

    > retStatus = ISD200_ERROR;
    > - }
    > + } else
    > + info->srb.sense_buffer = info->sense_buffer;
    > }
    >
    > if (retStatus == ISD200_GOOD) {

    James




    \
     
     \ /
      Last update: 2008-03-12 17:57    [W:0.026 / U:299.824 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site