mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Tejun Heo <htejun@gmail.com>
To: torvalds@osdl.org, axboe@suse.de, acme@mandriva.com
Cc: linux-kernel@vger.kernel.org
Subject: [PATCH] blk: fix dangling pointer access in __elv_add_request
Date: Tue, 1 Nov 2005 17:23:49 +0900	[thread overview]
Message-ID: <20051101082349.GA17756@htj.dyndns.org> (raw)

cfq's add_req_fn callback may invoke q->request_fn directly and
depending on low-level driver used and timing, a queued request may be
finished & deallocated before add_req_fn callback returns.  So,
__elv_add_request must not access rq after it's passed to add_req_fn
callback.

This patch moves rq_mergeable test above add_req_fn().  This may
result in q->last_merge pointing to REQ_NOMERGE request if add_req_fn
callback sets it but as RQ_NOMERGE is checked again when blk layer
actually tries to merge requests, this does not cause any problem.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---

Arnaldo, I think this patch should fix the oops you're seeing.  Please
let me know how it works.  And thanks again for detailed reporting.

Jens, does generalizing queue kicking functions and disallowing
ioscheds from directly calling q->request_fn sound like a good idea?

Linus, with or without Arnaldo's confirmation, this patch fixes an
existing bug.  Please apply.  Thanks.

diff --git a/drivers/block/elevator.c b/drivers/block/elevator.c
--- a/drivers/block/elevator.c
+++ b/drivers/block/elevator.c
@@ -369,9 +369,14 @@ void __elv_add_request(request_queue_t *
 	case ELEVATOR_INSERT_SORT:
 		BUG_ON(!blk_fs_request(rq));
 		rq->flags |= REQ_SORTED;
-		q->elevator->ops->elevator_add_req_fn(q, rq);
 		if (q->last_merge == NULL && rq_mergeable(rq))
 			q->last_merge = rq;
+		/*
+		 * Some ioscheds (cfq) run q->request_fn directly, so
+		 * rq cannot be accessed after calling
+		 * elevator_add_req_fn.
+		 */
+		q->elevator->ops->elevator_add_req_fn(q, rq);
 		break;
 
 	default:

             reply	other threads:[~2005-11-01  8:24 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-11-01  8:23 Tejun Heo [this message]
2005-11-01  9:08 ` Jens Axboe
2005-11-01 10:13   ` Tejun Heo
2005-11-01 12:15     ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20051101082349.GA17756@htj.dyndns.org \
    --to=htejun@gmail.com \
    --cc=acme@mandriva.com \
    --cc=axboe@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@osdl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome