From 52149fa5375819fb2281338c0643119ae62ba33f Mon Sep 17 00:00:00 2001 From: Sujit Reddy Thumma Date: Tue, 10 Jun 2014 17:57:52 +0530 Subject: [PATCH] mmc: core: Fix null pointer dereference due to race conditions Fix race condition between mmcqd thread and the mmc_queue_suspend updating a shared variable mq->flags, which can lead to potential null pointer dereference as following- Unable to handle kernel NULL pointer dereference at virtual address 00000020 pgd = c0004000 [00000020] *pgd=00000000 mmcqd/0: 186] Internal error: Oops: 5 [#1] PREEMPT SMP ARM CPU: 0 Tainted: G W (3.4.0-1251694-eng #1) PC is at mmc_blk_err_check+0x20c/0x3b8 LR is at mmc_start_req+0x198/0x718 cpu0 | cpu1 x |= 1 | x |= 2 final value of x can be x = 1 or x = 2 Signed-off-by: Sujit Reddy Thumma Signed-off-by: franciscofranco --- drivers/mmc/card/block.c | 13 +++++++------ drivers/mmc/card/queue.c | 20 +++++++++----------- drivers/mmc/card/queue.h | 8 ++++---- 3 files changed, 20 insertions(+), 21 deletions(-) diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c index 96c99a364d32..8db6b7a8ccc5 100644 --- a/drivers/mmc/card/block.c +++ b/drivers/mmc/card/block.c @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -2549,7 +2550,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc) areq = mmc_start_req(card->host, areq, (int *) &status); if (!areq) { if (status == MMC_BLK_NEW_REQUEST) - mq->flags |= MMC_QUEUE_NEW_REQUEST; + set_bit(MMC_QUEUE_NEW_REQUEST, &mq->flags); return 0; } @@ -2570,7 +2571,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc) mmc_blk_reinsert_req(areq); } - mq->flags |= MMC_QUEUE_URGENT_REQUEST; + set_bit(MMC_QUEUE_URGENT_REQUEST, &mq->flags); ret = 0; break; case MMC_BLK_URGENT_DONE: @@ -2751,8 +2752,8 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req) mmc_blk_write_packing_control(mq, req); - mq->flags &= ~MMC_QUEUE_NEW_REQUEST; - mq->flags &= ~MMC_QUEUE_URGENT_REQUEST; + clear_bit(MMC_QUEUE_NEW_REQUEST, &mq->flags); + clear_bit(MMC_QUEUE_URGENT_REQUEST, &mq->flags); if (cmd_flags & REQ_DISCARD) { /* complete ongoing async transfer before issuing discard */ if (card->host->areq) @@ -2783,8 +2784,8 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req) * - urgent notification in progress and current request is not urgent * (all existing requests completed or reinserted to the block layer) */ - if ((!req && !(mq->flags & MMC_QUEUE_NEW_REQUEST)) || - ((mq->flags & MMC_QUEUE_URGENT_REQUEST) && + if ((!req && !(test_bit(MMC_QUEUE_NEW_REQUEST, &mq->flags))) || + ((test_bit(MMC_QUEUE_URGENT_REQUEST, &mq->flags)) && !(mq->mqrq_cur->req->cmd_flags & MMC_REQ_NOREINSERT_MASK))) { if (mmc_card_need_bkops(card)) diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c index 47c629ee6b19..2bfc073ded5c 100644 --- a/drivers/mmc/card/queue.c +++ b/drivers/mmc/card/queue.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -82,12 +83,12 @@ static int mmc_queue_thread(void *d) set_current_state(TASK_RUNNING); cmd_flags = req ? req->cmd_flags : 0; mq->issue_fn(mq, req); - if (mq->flags & MMC_QUEUE_NEW_REQUEST) { + if (test_bit(MMC_QUEUE_NEW_REQUEST, &mq->flags)) { continue; /* fetch again */ - } else if ((mq->flags & MMC_QUEUE_URGENT_REQUEST) && - (mq->mqrq_cur->req && - !(mq->mqrq_cur->req->cmd_flags & - MMC_REQ_NOREINSERT_MASK))) { + } else if (test_bit(MMC_QUEUE_URGENT_REQUEST, + &mq->flags) && (mq->mqrq_cur->req && + !(mq->mqrq_cur->req->cmd_flags & + MMC_REQ_NOREINSERT_MASK))) { /* * clean current request when urgent request * processing in progress and current request is @@ -508,9 +509,7 @@ int mmc_queue_suspend(struct mmc_queue *mq, int wait) unsigned long flags; int rc = 0; - if (!(mq->flags & MMC_QUEUE_SUSPENDED)) { - mq->flags |= MMC_QUEUE_SUSPENDED; - + if (!(test_and_set_bit(MMC_QUEUE_SUSPENDED, &mq->flags))) { spin_lock_irqsave(q->queue_lock, flags); blk_stop_queue(q); spin_unlock_irqrestore(q->queue_lock, flags); @@ -521,7 +520,7 @@ int mmc_queue_suspend(struct mmc_queue *mq, int wait) * Failed to take the lock so better to abort the * suspend because mmcqd thread is processing requests. */ - mq->flags &= ~MMC_QUEUE_SUSPENDED; + clear_bit(MMC_QUEUE_SUSPENDED, &mq->flags); spin_lock_irqsave(q->queue_lock, flags); blk_start_queue(q); spin_unlock_irqrestore(q->queue_lock, flags); @@ -543,8 +542,7 @@ void mmc_queue_resume(struct mmc_queue *mq) struct request_queue *q = mq->queue; unsigned long flags; - if (mq->flags & MMC_QUEUE_SUSPENDED) { - mq->flags &= ~MMC_QUEUE_SUSPENDED; + if (test_and_clear_bit(MMC_QUEUE_SUSPENDED, &mq->flags)) { up(&mq->thread_sem); diff --git a/drivers/mmc/card/queue.h b/drivers/mmc/card/queue.h index b6dcf0962a71..bb8ab61943ee 100644 --- a/drivers/mmc/card/queue.h +++ b/drivers/mmc/card/queue.h @@ -47,10 +47,10 @@ struct mmc_queue { struct mmc_card *card; struct task_struct *thread; struct semaphore thread_sem; - unsigned int flags; -#define MMC_QUEUE_SUSPENDED (1 << 0) -#define MMC_QUEUE_NEW_REQUEST (1 << 1) -#define MMC_QUEUE_URGENT_REQUEST (1 << 2) + unsigned long flags; +#define MMC_QUEUE_SUSPENDED 0 +#define MMC_QUEUE_NEW_REQUEST 1 +#define MMC_QUEUE_URGENT_REQUEST 2 int (*issue_fn)(struct mmc_queue *, struct request *); void *data;