lkml.org 
[lkml]   [2012]   [Dec]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [PATCH 139/241] block: Don't access request after it might be freed
    From
    Date
    On Thu, 2012-12-13 at 11:58 -0200, Herton Ronaldo Krzesinski wrote:
    > 3.5.7.2 -stable review patch. If anyone has any objections, please let me know.
    >
    > ------------------
    >
    > From: Roland Dreier <roland@purestorage.com>
    >
    > commit 893d290f1d7496db97c9471bc352ad4a11dc8a25 upstream.
    >
    > After we've done __elv_add_request() and __blk_run_queue() in
    > blk_execute_rq_nowait(), the request might finish and be freed
    > immediately. Therefore checking if the type is REQ_TYPE_PM_RESUME
    > isn't safe afterwards, because if it isn't, rq might be gone.
    > Instead, check beforehand and stash the result in a temporary.
    >
    > This fixes crashes in blk_execute_rq_nowait() I get occasionally when
    > running with lots of memory debugging options enabled -- I think this
    > race is usually harmless because the window for rq to be reallocated
    > is so small.
    >
    > Signed-off-by: Roland Dreier <roland@purestorage.com>
    > Signed-off-by: Jens Axboe <axboe@kernel.dk>
    > [ herton: adjust context ]
    > Signed-off-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>

    This is missing from 3.{0,4} but I did apply it to 3.2, again with the
    need to adjust context.

    Perhaps the intermediate fixes to blk_execute_rq_nowait() that resulted
    in the changed context should also be applied to stable updates. The
    fixes in question are:

    8ba61435d73f block: add missing blk_queue_dead() checks
    - applied in 3.3; missing from 3.{0,2}.y
    e81ca6fe85b7 [SCSI] block: Fix blk_execute_rq_nowait() dead queue handling
    - applied in 3.6; missing from 3.{0,2,4,5}.y
    893d290f1d74 block: Don't access request after it might be freed
    - applied in 3.7, then to 3.{2,6}.y; missing from 3.{0,4}.y

    Jens?

    Ben.
    > ---
    > block/blk-exec.c | 10 +++++++++-
    > 1 file changed, 9 insertions(+), 1 deletion(-)
    >
    > diff --git a/block/blk-exec.c b/block/blk-exec.c
    > index fb2cbd5..9925fbe 100644
    > --- a/block/blk-exec.c
    > +++ b/block/blk-exec.c
    > @@ -49,8 +49,16 @@ void blk_execute_rq_nowait(struct request_queue *q, struct gendisk *bd_disk,
    > rq_end_io_fn *done)
    > {
    > int where = at_head ? ELEVATOR_INSERT_FRONT : ELEVATOR_INSERT_BACK;
    > + bool is_pm_resume;
    >
    > WARN_ON(irqs_disabled());
    > +
    > + /*
    > + * need to check this before __blk_run_queue(), because rq can
    > + * be freed before that returns.
    > + */
    > + is_pm_resume = rq->cmd_type == REQ_TYPE_PM_RESUME;
    > +
    > spin_lock_irq(q->queue_lock);
    >
    > if (unlikely(blk_queue_dead(q))) {
    > @@ -66,7 +74,7 @@ void blk_execute_rq_nowait(struct request_queue *q, struct gendisk *bd_disk,
    > __elv_add_request(q, rq, where);
    > __blk_run_queue(q);
    > /* the queue is stopped so it won't be run */
    > - if (rq->cmd_type == REQ_TYPE_PM_RESUME)
    > + if (is_pm_resume)
    > q->request_fn(q);
    > spin_unlock_irq(q->queue_lock);
    > }

    --
    Ben Hutchings
    Theory and practice are closer in theory than in practice.
    - John Levine, moderator of comp.compilers
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2012-12-15 21:21    [W:4.621 / U:0.036 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site