Messages in this thread | | | From | Kees Cook <> | Date | Tue, 17 Apr 2018 13:46:04 -0700 | Subject | Re: usercopy whitelist woe in scsi_sense_cache |
| |
On Tue, Apr 17, 2018 at 1:28 PM, Jens Axboe <axboe@kernel.dk> wrote: > It has to be the latter bfqq->dispatched increment, as those are > transient (and bfqd is not).
Yeah, and I see a lot of comments around the lifetime of rq and bfqq, so I assume something is not being locked correctly.
#define RQ_BFQQ(rq) ((rq)->elv.priv[1])
static struct request *__bfq_dispatch_request(struct blk_mq_hw_ctx *hctx) { struct bfq_data *bfqd = hctx->queue->elevator->elevator_data; struct request *rq = NULL; struct bfq_queue *bfqq = NULL;
if (!list_empty(&bfqd->dispatch)) { rq = list_first_entry(&bfqd->dispatch, struct request, queuelist); list_del_init(&rq->queuelist);
bfqq = RQ_BFQQ(rq);
if (bfqq) { /* * Increment counters here, because this * dispatch does not follow the standard * dispatch flow (where counters are * incremented) */ bfqq->dispatched++; ...
I see elv.priv[1] assignments made in a few places -- is it possible there is some kind of uninitialized-but-not-NULL state that can leak in there?
bfq_prepare_request() assigns elv.priv[1], and bfq_insert_request() only checks that it's non-NULL (if at all) in one case. Can bfq_insert_request() get called without bfq_prepare_request() being called first?
-Kees
-- Kees Cook Pixel Security
| |