lkml.org 
[lkml]   [2011]   [Jul]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: Linux 3.0 oopses when pulling a USB CDROM
    From
    On Sat, Jul 2, 2011 at 12:59 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
    > On Sat, 2 Jul 2011, Andi Kleen wrote:
    >
    >> > The problem is that blk_peek_request() calls scsi_prep_fn(), which
    >> > does this:
    >> >
    >> >     struct scsi_device *sdev = q->queuedata;
    >> >     int ret = BLKPREP_KILL;
    >> >
    >> >     if (req->cmd_type == REQ_TYPE_BLOCK_PC)
    >> >             ret = scsi_setup_blk_pc_cmnd(sdev, req);
    >> >     return scsi_prep_return(q, req, ret);
    >> >
    >> > It doesn't check to see if sdev is NULL, nor does
    >> > scsi_setup_blk_pc_cmnd().  That accounts for this error:
    >>
    >> I actually added a NULL check in scsi_setup_blk_pc_cmnd early on,
    >> but that just caused RCU CPU stalls afterwards and then eventually
    >> a hung system.
    >
    > The RCU problem is likely to be a separate issue.  It might even be a
    > result of the use-after-free problem with the elevator.
    >
    > At any rate, it's clear that the crash in the refcounting log you
    > posted occurred because scsi_setup_blk_pc_cmnd() called
    > scsi_prep_state_check(), which tried to dereference the NULL pointer.
    >
    > Would you like to try this patch to see if it fixes the problem?  As I
    > said before, I'm not certain it's the best thing to do, but it worked
    > on my system.
    >
    > Alan Stern
    >
    >
    >
    >
    > Index: usb-3.0/drivers/scsi/scsi_lib.c
    > ===================================================================
    > --- usb-3.0.orig/drivers/scsi/scsi_lib.c
    > +++ usb-3.0/drivers/scsi/scsi_lib.c
    > @@ -1247,6 +1247,8 @@ int scsi_prep_fn(struct request_queue *q
    >        struct scsi_device *sdev = q->queuedata;
    >        int ret = BLKPREP_KILL;
    >
    > +       if (!sdev)
    > +               return ret;
    >        if (req->cmd_type == REQ_TYPE_BLOCK_PC)
    >                ret = scsi_setup_blk_pc_cmnd(sdev, req);
    >        return scsi_prep_return(q, req, ret);
    > Index: usb-3.0/drivers/scsi/scsi_sysfs.c
    > ===================================================================
    > --- usb-3.0.orig/drivers/scsi/scsi_sysfs.c
    > +++ usb-3.0/drivers/scsi/scsi_sysfs.c
    > @@ -322,6 +322,8 @@ static void scsi_device_dev_release_user
    >                kfree(evt);
    >        }
    >
    > +       /* Freeing the queue signals to block that we're done */
    > +       scsi_free_queue(sdev->request_queue);
    >        blk_put_queue(sdev->request_queue);
    >        /* NULL queue means the device can't be used */
    >        sdev->request_queue = NULL;
    > @@ -936,8 +938,6 @@ void __scsi_remove_device(struct scsi_de
    >        /* cause the request function to reject all I/O requests */
    >        sdev->request_queue->queuedata = NULL;
    >
    > -       /* Freeing the queue signals to block that we're done */
    > -       scsi_free_queue(sdev->request_queue);
    >        put_device(dev);
    >  }

    This patch seems to resolve the block/scsi null-ptr de-references in
    our libsas/isci environment, we have yet to try James' alternative
    [1]. Do we potentially need both?

    Commit 86cbfb56 moved scsi_free_queue to __scsi_remove_device() but it
    seems only the "sdev->request_queue->queuedata = NULL" needed to be
    moved?

    The conversation appeared to be awaiting test results...

    [1]: http://marc.info/?l=linux-scsi&m=131007155700831&w=2

    --
    Dan
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2011-07-18 19:01    [W:0.024 / U:2.976 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site