lkml.org 
[lkml]   [2009]   [Dec]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [osd-dev] [PATCH] scsi_lib: Bug in completion of bidi commands
    On 12/15/2009 05:25 PM, Boaz Harrosh wrote:
    >
    > Because of the terrible structuring of scsi-bidi-commands
    > it breaks some of the life time rules of a scsi-command.
    > It is now not allowed to free up the block-request before
    > cleanup and partial deallocation of the scsi-command. (Which
    > is not so for none bidi commands)
    >
    > The right fix to this problem would be to make bidi command
    > a first citizen by allocating a scsi_sdb pointer at scsi command
    > just like cmd->prot_sdb. The bidi sdb should be allocated/deallocated
    > as part of the get/put_command (Again like the prot_sdb) and the
    > current decoupling of scsi_cmnd and blk-request should be kept.
    >
    > For now make sure scsi_release_buffers() is called before the
    > call to blk_end_request_all() which might cause the suicide of
    > the block requests. At best the leak of bidi buffers, at worse
    > a crash, as there is a race between the existence of the bidi_request
    > and the free of the associated bidi_sdb.
    >
    > The reason this was never hit before is because only OSD has the potential
    > of doing asynchronous bidi commands. (So does bsg but it is never used)
    > And OSD clients just happen to do all their bidi commands synchronously, up
    > until recently.
    >
    > CC: Stable Tree <stable@kernel.org>
    > Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>

    James hi.

    What about this BUG. It affects anybody doing bidi commands. The possibilities
    are an sglist leak at best, and a crash at worse.

    I understand this code needs cleanup, but first things first. Lets first fix the
    bug, which should also go to stable. Then the cleanup can go to next merge window.

    BTW: Should I attempt a cleanup on current code, or should I wait for Alan's Patch
    to go in first?

    Thanks
    Boaz

    > ---
    > drivers/scsi/scsi_lib.c | 2 +-
    > 1 files changed, 1 insertions(+), 1 deletions(-)
    >
    > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
    > index 5987da8..bc9a881 100644
    > --- a/drivers/scsi/scsi_lib.c
    > +++ b/drivers/scsi/scsi_lib.c
    > @@ -749,9 +749,9 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
    > */
    > req->next_rq->resid_len = scsi_in(cmd)->resid;
    >
    > + scsi_release_buffers(cmd);
    > blk_end_request_all(req, 0);
    >
    > - scsi_release_buffers(cmd);
    > scsi_next_command(cmd);
    > return;
    > }



    \
     
     \ /
      Last update: 2009-12-31 08:55    [W:0.025 / U:0.704 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site