[PATCH] blk: fix dangling pointer access in __elv_add_request

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <[email protected]>
---

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:
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux