linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vladimir Oltean <olteanv@gmail.com>
To: broonie@kernel.org
Cc: linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org,
	shawnguo@kernel.org, robh+dt@kernel.org, mark.rutland@arm.com,
	devicetree@vger.kernel.org, eha@deif.com, angelo@sysam.it,
	andrew.smirnov@gmail.com, gustavo@embeddedor.com,
	weic@nvidia.com, mhosny@nvidia.com, michael@walle.cc,
	peng.ma@nxp.com
Subject: [PATCH v3 2/7] spi: spi-fsl-dspi: Avoid use-after-free in interrupt mode
Date: Tue, 10 Mar 2020 14:55:37 +0200	[thread overview]
Message-ID: <20200310125542.5939-3-olteanv@gmail.com> (raw)
In-Reply-To: <20200310125542.5939-1-olteanv@gmail.com>

From: Vladimir Oltean <vladimir.oltean@nxp.com>

When the wait_event_interruptible call fails, there is still a chance
that the dspi_interrupt may arrive (for example the kernel thread doing
the SPI pump really is interrupted).

In that case, dspi_transfer_one_message will return execution all the
way to the spi_device driver, which may free the spi_message and
spi_transfer structures.

But when the interrupt arrives, the driver still accesses those
structures, leading to use-after-free issues as can be seen below:

hexdump -C /dev/mtd0
00000000  00 75 68 75 0a ff ff ff  ff ff ff ff ff ff ff ff
|.uhu............|
00000010  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff
|................|
*
^C[   38.495955] fsl-dspi 2120000.spi: Waiting for transfer to complete failed!
[   38.503097] spi_master spi2: failed to transfer one message from queue
[   38.509729] Unable to handle kernel paging request at virtual address ffff800095ab3377
[   38.517676] Mem abort info:
[   38.520474]   ESR = 0x96000045
[   38.523533]   EC = 0x25: DABT (current EL), IL = 32 bits
[   38.528861]   SET = 0, FnV = 0
[   38.531921]   EA = 0, S1PTW = 0
[   38.535067] Data abort info:
[   38.537952]   ISV = 0, ISS = 0x00000045
[   38.541797]   CM = 0, WnR = 1
[   38.544771] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000082621000
[   38.551494] [ffff800095ab3377] pgd=00000020fffff003, p4d=00000020fffff003, pud=0000000000000000
[   38.560229] Internal error: Oops: 96000045 [#1] PREEMPT SMP
[   38.565819] Modules linked in:
[   38.568882] CPU: 0 PID: 2729 Comm: hexdump Not tainted 5.6.0-rc4-next-20200306-00052-gd8730cdc8a0b-dirty #193
[   38.578834] Hardware name: Kontron SMARC-sAL28 (Single PHY) on SMARC Eval 2.0 carrier (DT)
[   38.587129] pstate: 20000085 (nzCv daIf -PAN -UAO)
[   38.591941] pc : ktime_get_real_ts64+0x3c/0x110
[   38.596487] lr : spi_take_timestamp_pre+0x40/0x90
[   38.601203] sp : ffff800010003d90
[   38.604525] x29: ffff800010003d90 x28: ffff80001200e000
[   38.609854] x27: ffff800011da9000 x26: ffff002079c40400
[   38.615184] x25: ffff8000117fe018 x24: ffff800011daa1a0
[   38.620513] x23: ffff800015ab3860 x22: ffff800095ab3377
[   38.625841] x21: 000000000000146e x20: ffff8000120c3000
[   38.631170] x19: ffff0020795f6e80 x18: ffff800011da9948
[   38.636498] x17: 0000000000000000 x16: 0000000000000000
[   38.641826] x15: ffff800095ab3377 x14: 0720072007200720
[   38.647155] x13: 0720072007200765 x12: 0775076507750771
[   38.652483] x11: 0720076d076f0772 x10: 0000000000000040
[   38.657812] x9 : ffff8000108e2100 x8 : ffff800011dcabe8
[   38.663139] x7 : 0000000000000000 x6 : ffff800015ab3a60
[   38.668468] x5 : 0000000007200720 x4 : ffff800095ab3377
[   38.673796] x3 : 0000000000000000 x2 : 0000000000000ab0
[   38.679125] x1 : ffff800011daa000 x0 : 0000000000000026
[   38.684454] Call trace:
[   38.686905]  ktime_get_real_ts64+0x3c/0x110
[   38.691100]  spi_take_timestamp_pre+0x40/0x90
[   38.695470]  dspi_fifo_write+0x58/0x2c0
[   38.699315]  dspi_interrupt+0xbc/0xd0
[   38.702987]  __handle_irq_event_percpu+0x78/0x2c0
[   38.707706]  handle_irq_event_percpu+0x3c/0x90
[   38.712161]  handle_irq_event+0x4c/0xd0
[   38.716008]  handle_fasteoi_irq+0xbc/0x170
[   38.720115]  generic_handle_irq+0x2c/0x40
[   38.724135]  __handle_domain_irq+0x68/0xc0
[   38.728243]  gic_handle_irq+0xc8/0x160
[   38.732000]  el1_irq+0xb8/0x180
[   38.735149]  spi_nor_spimem_read_data+0xe0/0x140
[   38.739779]  spi_nor_read+0xc4/0x120
[   38.743364]  mtd_read_oob+0xa8/0xc0
[   38.746860]  mtd_read+0x4c/0x80
[   38.750007]  mtdchar_read+0x108/0x2a0
[   38.753679]  __vfs_read+0x20/0x50
[   38.757002]  vfs_read+0xa4/0x190
[   38.760237]  ksys_read+0x6c/0xf0
[   38.763471]  __arm64_sys_read+0x20/0x30
[   38.767319]  el0_svc_common.constprop.3+0x90/0x160
[   38.772125]  do_el0_svc+0x28/0x90
[   38.775449]  el0_sync_handler+0x118/0x190
[   38.779468]  el0_sync+0x140/0x180
[   38.782793] Code: 91000294 1400000f d50339bf f9405e80 (f90002c0)
[   38.788910] ---[ end trace 55da560db4d6bef7 ]---
[   38.793540] Kernel panic - not syncing: Fatal exception in interrupt
[   38.799914] SMP: stopping secondary CPUs
[   38.803849] Kernel Offset: disabled
[   38.807344] CPU features: 0x10002,20006008
[   38.811451] Memory Limit: none
[   38.814513] ---[ end Kernel panic - not syncing: Fatal exception in interrupt ]---

The solution is to mask interrupts when we know we can't deal with them.
Also, to make sure they won't bite when we enable them back, we clear
the status register such that stale interrupt requests will just be lost
(as is expected on timeout or wait interruption).

Fixes: 349ad66c0ab0 ("spi:Add Freescale DSPI driver for Vybrid VF610 platform")
Reported-by: Michael Walle <michael@walle.cc>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v3:
Patch is new.

 drivers/spi/spi-fsl-dspi.c | 63 +++++++++++++++++++++++---------------
 1 file changed, 38 insertions(+), 25 deletions(-)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 0ce26c1cbf62..f2ba0731aebe 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -911,15 +911,39 @@ static irqreturn_t dspi_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static void dspi_enable_interrupts(struct fsl_dspi *dspi, bool on)
+{
+	u32 spi_rser = 0;
+
+	if (on) {
+		switch (dspi->devtype_data->trans_mode) {
+		case DSPI_EOQ_MODE:
+			spi_rser = SPI_RSER_EOQFE;
+			break;
+		case DSPI_XSPI_MODE:
+			spi_rser = SPI_RSER_CMDTCFE;
+			break;
+		default:
+			/* Interrupts not necessary for DMA mode */
+			return;
+		}
+	}
+
+	regmap_write(dspi->regmap, SPI_SR, SPI_SR_CLEAR);
+	regmap_write(dspi->regmap, SPI_RSER, spi_rser);
+}
+
 static int dspi_transfer_one_message(struct spi_controller *ctlr,
 				     struct spi_message *message)
 {
 	struct fsl_dspi *dspi = spi_controller_get_devdata(ctlr);
 	struct spi_device *spi = message->spi;
-	enum dspi_trans_mode trans_mode;
 	struct spi_transfer *transfer;
 	int status = 0;
 
+	if (dspi->irq)
+		dspi_enable_interrupts(dspi, true);
+
 	message->actual_length = 0;
 
 	list_for_each_entry(transfer, &message->transfers, transfer_list) {
@@ -965,37 +989,24 @@ static int dspi_transfer_one_message(struct spi_controller *ctlr,
 		spi_take_timestamp_pre(dspi->ctlr, dspi->cur_transfer,
 				       dspi->progress, !dspi->irq);
 
-		trans_mode = dspi->devtype_data->trans_mode;
-		switch (trans_mode) {
-		case DSPI_EOQ_MODE:
-			regmap_write(dspi->regmap, SPI_RSER, SPI_RSER_EOQFE);
-			dspi_fifo_write(dspi);
-			break;
-		case DSPI_XSPI_MODE:
-			regmap_write(dspi->regmap, SPI_RSER, SPI_RSER_CMDTCFE);
-			dspi_fifo_write(dspi);
-			break;
-		case DSPI_DMA_MODE:
+		if (dspi->devtype_data->trans_mode == DSPI_DMA_MODE) {
 			regmap_write(dspi->regmap, SPI_RSER,
 				     SPI_RSER_TFFFE | SPI_RSER_TFFFD |
 				     SPI_RSER_RFDFE | SPI_RSER_RFDFD);
 			status = dspi_dma_xfer(dspi);
-			break;
-		default:
-			dev_err(&dspi->pdev->dev, "unsupported trans_mode %u\n",
-				trans_mode);
-			status = -EINVAL;
-			goto out;
-		}
+			if (status)
+				goto out;
+		} else if (dspi->irq) {
+			/* Kick off the interrupt train */
+			dspi_fifo_write(dspi);
 
-		if (!dspi->irq) {
-			do {
-				status = dspi_poll(dspi);
-			} while (status == -EINPROGRESS);
-		} else if (trans_mode != DSPI_DMA_MODE) {
 			status = wait_event_interruptible(dspi->waitq,
 							  dspi->waitflags);
 			dspi->waitflags = 0;
+		} else {
+			do {
+				status = dspi_poll(dspi);
+			} while (status == -EINPROGRESS);
 		}
 		if (status)
 			dev_err(&dspi->pdev->dev,
@@ -1005,6 +1016,9 @@ static int dspi_transfer_one_message(struct spi_controller *ctlr,
 	}
 
 out:
+	if (dspi->irq)
+		dspi_enable_interrupts(dspi, false);
+
 	message->status = status;
 	spi_finalize_current_message(ctlr);
 
@@ -1208,7 +1222,6 @@ static void dspi_init(struct fsl_dspi *dspi)
 		mcr |= SPI_MCR_MASTER;
 
 	regmap_write(dspi->regmap, SPI_MCR, mcr);
-	regmap_write(dspi->regmap, SPI_SR, SPI_SR_CLEAR);
 }
 
 static int dspi_slave_abort(struct spi_master *master)
-- 
2.17.1

  reply	other threads:[~2020-03-10 12:55 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-10 12:55 [PATCH v3 0/7] NXP DSPI bugfixes and support for LS1028A Vladimir Oltean
2020-03-10 12:55 ` Vladimir Oltean [this message]
2020-03-10 12:55 ` [PATCH v3 5/7] spi: spi-fsl-dspi: Add " Vladimir Oltean
2020-03-10 12:55 ` [PATCH v3 6/7] arm64: dts: ls1028a: Specify the DMA channels for the DSPI controllers Vladimir Oltean
     [not found] ` <20200310125542.5939-1-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2020-03-10 12:55   ` [PATCH v3 1/7] spi: spi-fsl-dspi: Don't access reserved fields in SPI_MCR Vladimir Oltean
2020-03-10 12:55   ` [PATCH v3 3/7] spi: spi-fsl-dspi: Fix little endian access to PUSHR CMD and TXDATA Vladimir Oltean
2020-03-10 12:55   ` [PATCH v3 4/7] spi: spi-fsl-dspi: Fix bits-per-word acceleration in DMA mode Vladimir Oltean
2020-03-10 12:55   ` [PATCH v3 7/7] arm64: dts: ls1028a-rdb: Add a spidev node for the mikroBUS Vladimir Oltean
2020-03-10 14:11   ` [PATCH v3 0/7] NXP DSPI bugfixes and support for LS1028A Michael Walle
     [not found]     ` <615284875b709f602d57e4a4621a83c1-QKn5cuLxLXY@public.gmane.org>
2020-03-10 14:56       ` Vladimir Oltean
2020-03-10 15:22         ` Michael Walle
     [not found]           ` <59b07b7d70603c6b536a7354ed0ea8d8-QKn5cuLxLXY@public.gmane.org>
2020-03-13 16:07             ` Michael Walle
2020-03-13 16:37               ` Vladimir Oltean
     [not found]                 ` <CA+h21hqk+pVrGgHx4iTshfE3i4WF7VANPfMf2ykPFpL3=ragag-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2020-03-13 16:53                   ` Michael Walle

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=20200310125542.5939-3-olteanv@gmail.com \
    --to=olteanv@gmail.com \
    --cc=andrew.smirnov@gmail.com \
    --cc=angelo@sysam.it \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=eha@deif.com \
    --cc=gustavo@embeddedor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mhosny@nvidia.com \
    --cc=michael@walle.cc \
    --cc=peng.ma@nxp.com \
    --cc=robh+dt@kernel.org \
    --cc=shawnguo@kernel.org \
    --cc=weic@nvidia.com \
    --subject='Re: [PATCH v3 2/7] spi: spi-fsl-dspi: Avoid use-after-free in interrupt mode' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).