Analyzing the KASAN report [1] tells us that in
mmc_request_done+0xcc/0x30c
what can be resolved to an access to
mrq->cap_cmd_during_tfr
in mmc_command_done() called inline from mmc_request_done() "mrq"
becomes invalid.
In the driver we have two work queues, tmio_mmc_reset_work() and
tmio_mmc_done_work(). Both operate on mrq.
Synchronize this by extending the spin lock protected sections.
[1]
BUG: KASAN: stack-out-of-bounds in mmc_request_done+0xcc/0x30c
Read of size 1 at addr ffff8005df517738 by task kworker/4:1/2308
CPU: 4 PID: 2308 Comm: kworker/4:1 Tainted: G C 4.14.327-ltsi
Hardware name: custom board based on r8a7796 (DT)
Workqueue: events tmio_mmc_reset_work
Call trace:
dump_backtrace+0x0/0x1fc
show_stack+0x14/0x1c
dump_stack+0xcc/0x114
print_address_description+0x54/0x238
kasan_report+0x274/0x29c
__asan_load1+0x24/0x50
mmc_request_done+0xcc/0x30c
tmio_mmc_reset_work+0x1fc/0x248
process_one_work+0x324/0x6c0
worker_thread+0x358/0x4d4
kthread+0x1a4/0x1bc
ret_from_fork+0x10/0x18
The buggy address belongs to the page:
page:ffff7e00177d45c0 pfn:61f517 refcount:0 mapcount:0 mapping:
(null) index:0x0
flags: 0x4000000000000000()
raw: 4000000000000000 0000000000000000 0000000000000000 00000000ffffffff
raw: 0000000000000000 ffff7e00177d45e0 0000000000000000 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8005df517600: 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00 00 00
ffff8005df517680: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
>ffff8005df517700: 00 00 f1 f1 f1 f1 04 f2 f2 f2 00 00 00 00 00 00
^
ffff8005df517780: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff8005df517800: 00 00 f1 f1 f1 f1 04 f2 f2 f2 f2 f2 f2 f2 00 00
==================================================================
Disabling lock debugging due to kernel taint
------------[ cut here ]------------
Cc: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Dirk Behme <dirk.behme@xxxxxxxxxxxx>
---
Notes: Instead of extending the irqsave spin lock sections and with
this blocking the interrupts even longer we could discuss if
a new "mrq synchronization" spin lock should be added. But of
course the 'host->mrq = NULL;' needs to go into the host->lock
protected section.
drivers/mmc/host/tmio_mmc_core.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index be7f18fd4836..d876af57db48 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -261,14 +261,15 @@ static void tmio_mmc_reset_work(struct work_struct *work)
host->cmd = NULL;
host->data = NULL;
-
+ /* Ready for new calls */
+ host->mrq = NULL;
spin_unlock_irqrestore(&host->lock, flags);
tmio_mmc_reset(host, true);
- /* Ready for new calls */
- host->mrq = NULL;
+ spin_lock_irqsave(&host->lock, flags);
mmc_request_done(host->mmc, mrq);
+ spin_unlock_irqrestore(&host->lock, flags);
}
/* These are the bitmasks the tmio chip requires to implement the MMC response
@@ -812,9 +813,9 @@ static void tmio_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
wmb();
host->mrq = mrq;
- spin_unlock_irqrestore(&host->lock, flags);
-
tmio_process_mrq(host, mrq);
+
+ spin_unlock_irqrestore(&host->lock, flags);
}
static void tmio_mmc_finish_request(struct tmio_mmc_host *host)
@@ -841,8 +842,6 @@ static void tmio_mmc_finish_request(struct tmio_mmc_host *host)
cancel_delayed_work(&host->delayed_reset_work);
- spin_unlock_irqrestore(&host->lock, flags);
-
if (mrq->cmd->error || (mrq->data && mrq->data->error)) {
tmio_mmc_ack_mmc_irqs(host, TMIO_MASK_IRQ); /* Clear all */
tmio_mmc_abort_dma(host);
@@ -855,6 +854,7 @@ static void tmio_mmc_finish_request(struct tmio_mmc_host *host)
/* If SET_BLOCK_COUNT, continue with main command */
if (host->mrq && !mrq->cmd->error) {
tmio_process_mrq(host, mrq);
+ spin_unlock_irqrestore(&host->lock, flags);
return;
}
@@ -862,6 +862,7 @@ static void tmio_mmc_finish_request(struct tmio_mmc_host *host)
host->fixup_request(host, mrq);
mmc_request_done(host->mmc, mrq);
+ spin_unlock_irqrestore(&host->lock, flags);
}
static void tmio_mmc_done_work(struct work_struct *work)
--
2.28.0
