* [PATCH 1/5] spi: spi-zynqmp-gqspi: fix clk_enable/disable imbalance issue
2021-04-16 0:46 [PATCH 0/5] spi: spi-zynqmp-gqspi: fix spi issues quanyang.wang
@ 2021-04-16 0:46 ` quanyang.wang
2021-04-16 12:55 ` Mark Brown
2021-04-16 0:46 ` [PATCH 2/5] spi: spi-zynqmp-gqspi: fix hang issue when suspend/resume quanyang.wang
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: quanyang.wang @ 2021-04-16 0:46 UTC (permalink / raw)
To: Mark Brown, Michal Simek, Amit Kumar Mahapatra
Cc: linux-spi, linux-arm-kernel, linux-kernel, Quanyang Wang
From: Quanyang Wang <quanyang.wang@windriver.com>
The clks "pclk" and "ref_clk" are enabled twice during the probe. The
first time is in the function zynqmp_qspi_probe and the second time is
in zynqmp_qspi_setup_op which is called by devm_spi_register_controller.
Then calling zynqmp_qspi_remove (rmmod this module) to disable these clks
will trigger a warning as below:
[ 309.124604] Unpreparing enabled qspi_ref
[ 309.128641] WARNING: CPU: 1 PID: 537 at drivers/clk/clk.c:824 clk_core_unprepare+0x108/0x110
Since pm_runtime works now, clks can be enabled/disabled by calling
zynqmp_runtime_suspend/resume. So we don't need to enable these clks
explicitly in zynqmp_qspi_setup_op. Remove them to fix this issue.
And remove clk enabling/disabling in zynqmp_qspi_resume because there is
no spi transfer operation so enabling ref_clk is redundant meanwhile pclk
is not disabled for it is shared with other peripherals.
Furthermore replace clk_enable/disable with clk_prepare_enable and
clk_disable_unprepare in runtime_suspend/resume functions.
Fixes: 1c26372e5aa9 ("spi: spi-zynqmp-gqspi: Update driver to use spi-mem framework")
Signed-off-by: Quanyang Wang <quanyang.wang@windriver.com>
---
drivers/spi/spi-zynqmp-gqspi.c | 47 ++++++----------------------------
1 file changed, 8 insertions(+), 39 deletions(-)
diff --git a/drivers/spi/spi-zynqmp-gqspi.c b/drivers/spi/spi-zynqmp-gqspi.c
index 32e53f379e9b..f9056f0a480c 100644
--- a/drivers/spi/spi-zynqmp-gqspi.c
+++ b/drivers/spi/spi-zynqmp-gqspi.c
@@ -487,24 +487,10 @@ static int zynqmp_qspi_setup_op(struct spi_device *qspi)
{
struct spi_controller *ctlr = qspi->master;
struct zynqmp_qspi *xqspi = spi_controller_get_devdata(ctlr);
- struct device *dev = &ctlr->dev;
- int ret;
if (ctlr->busy)
return -EBUSY;
- ret = clk_enable(xqspi->refclk);
- if (ret) {
- dev_err(dev, "Cannot enable device clock.\n");
- return ret;
- }
-
- ret = clk_enable(xqspi->pclk);
- if (ret) {
- dev_err(dev, "Cannot enable APB clock.\n");
- clk_disable(xqspi->refclk);
- return ret;
- }
zynqmp_gqspi_write(xqspi, GQSPI_EN_OFST, GQSPI_EN_MASK);
return 0;
@@ -863,26 +849,9 @@ static int __maybe_unused zynqmp_qspi_suspend(struct device *dev)
static int __maybe_unused zynqmp_qspi_resume(struct device *dev)
{
struct spi_controller *ctlr = dev_get_drvdata(dev);
- struct zynqmp_qspi *xqspi = spi_controller_get_devdata(ctlr);
- int ret = 0;
-
- ret = clk_enable(xqspi->pclk);
- if (ret) {
- dev_err(dev, "Cannot enable APB clock.\n");
- return ret;
- }
-
- ret = clk_enable(xqspi->refclk);
- if (ret) {
- dev_err(dev, "Cannot enable device clock.\n");
- clk_disable(xqspi->pclk);
- return ret;
- }
spi_controller_resume(ctlr);
- clk_disable(xqspi->refclk);
- clk_disable(xqspi->pclk);
return 0;
}
@@ -898,8 +867,8 @@ static int __maybe_unused zynqmp_runtime_suspend(struct device *dev)
{
struct zynqmp_qspi *xqspi = (struct zynqmp_qspi *)dev_get_drvdata(dev);
- clk_disable(xqspi->refclk);
- clk_disable(xqspi->pclk);
+ clk_disable_unprepare(xqspi->refclk);
+ clk_disable_unprepare(xqspi->pclk);
return 0;
}
@@ -917,16 +886,16 @@ static int __maybe_unused zynqmp_runtime_resume(struct device *dev)
struct zynqmp_qspi *xqspi = (struct zynqmp_qspi *)dev_get_drvdata(dev);
int ret;
- ret = clk_enable(xqspi->pclk);
+ ret = clk_prepare_enable(xqspi->pclk);
if (ret) {
dev_err(dev, "Cannot enable APB clock.\n");
return ret;
}
- ret = clk_enable(xqspi->refclk);
+ ret = clk_prepare_enable(xqspi->refclk);
if (ret) {
dev_err(dev, "Cannot enable device clock.\n");
- clk_disable(xqspi->pclk);
+ clk_disable_unprepare(xqspi->pclk);
return ret;
}
@@ -1136,13 +1105,11 @@ static int zynqmp_qspi_probe(struct platform_device *pdev)
goto remove_master;
}
- init_completion(&xqspi->data_completion);
-
xqspi->refclk = devm_clk_get(&pdev->dev, "ref_clk");
if (IS_ERR(xqspi->refclk)) {
dev_err(dev, "ref_clk clock not found.\n");
ret = PTR_ERR(xqspi->refclk);
- goto clk_dis_pclk;
+ goto remove_master;
}
ret = clk_prepare_enable(xqspi->pclk);
@@ -1157,6 +1124,8 @@ static int zynqmp_qspi_probe(struct platform_device *pdev)
goto clk_dis_pclk;
}
+ init_completion(&xqspi->data_completion);
+
mutex_init(&xqspi->op_lock);
pm_runtime_use_autosuspend(&pdev->dev);
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/5] spi: spi-zynqmp-gqspi: fix clk_enable/disable imbalance issue
2021-04-16 0:46 ` [PATCH 1/5] spi: spi-zynqmp-gqspi: fix clk_enable/disable imbalance issue quanyang.wang
@ 2021-04-16 12:55 ` Mark Brown
2021-04-16 14:04 ` quanyang.wang
0 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2021-04-16 12:55 UTC (permalink / raw)
To: quanyang.wang
Cc: Michal Simek, Amit Kumar Mahapatra, linux-spi, linux-arm-kernel,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 691 bytes --]
On Fri, Apr 16, 2021 at 08:46:48AM +0800, quanyang.wang@windriver.com wrote:
> Since pm_runtime works now, clks can be enabled/disabled by calling
> zynqmp_runtime_suspend/resume. So we don't need to enable these clks
> explicitly in zynqmp_qspi_setup_op. Remove them to fix this issue.
> Fixes: 1c26372e5aa9 ("spi: spi-zynqmp-gqspi: Update driver to use spi-mem framework")
Are you *sure* this fixes is accurate? The patch (and several of the
others that flag the same commit) doesn't apply against for-5.12, though
at this point there's not really enough time to send another pull request
so it doesn't super matter though someone will probably need to help out
with stable backports.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/5] spi: spi-zynqmp-gqspi: fix clk_enable/disable imbalance issue
2021-04-16 12:55 ` Mark Brown
@ 2021-04-16 14:04 ` quanyang.wang
2021-04-16 15:12 ` Mark Brown
0 siblings, 1 reply; 10+ messages in thread
From: quanyang.wang @ 2021-04-16 14:04 UTC (permalink / raw)
To: Mark Brown
Cc: Michal Simek, Amit Kumar Mahapatra, linux-spi, linux-arm-kernel,
linux-kernel
Hi Mark,
On 4/16/21 8:55 PM, Mark Brown wrote:
> On Fri, Apr 16, 2021 at 08:46:48AM +0800, quanyang.wang@windriver.com wrote:
>
>> Since pm_runtime works now, clks can be enabled/disabled by calling
>> zynqmp_runtime_suspend/resume. So we don't need to enable these clks
>> explicitly in zynqmp_qspi_setup_op. Remove them to fix this issue.
>> Fixes: 1c26372e5aa9 ("spi: spi-zynqmp-gqspi: Update driver to use spi-mem framework")
> Are you *sure* this fixes is accurate? The patch (and several of the
> others that flag the same commit) doesn't apply against for-5.12, though
> at this point there's not really enough time to send another pull request
> so it doesn't super matter though someone will probably need to help out
> with stable backports.
I am sorry. These patches should NOT be with "Fixes" tag since they base
on the patches
which are not with "Fixes". May I send a V2 patch series which remove
these "Fixes" tags?
Thanks,
Quanyang
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/5] spi: spi-zynqmp-gqspi: fix clk_enable/disable imbalance issue
2021-04-16 14:04 ` quanyang.wang
@ 2021-04-16 15:12 ` Mark Brown
2021-04-16 15:37 ` Quanyang Wang
0 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2021-04-16 15:12 UTC (permalink / raw)
To: quanyang.wang
Cc: Michal Simek, Amit Kumar Mahapatra, linux-spi, linux-arm-kernel,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 489 bytes --]
On Fri, Apr 16, 2021 at 10:04:30PM +0800, quanyang.wang wrote:
> I am sorry. These patches should NOT be with "Fixes" tag since they base on
> the patches
> which are not with "Fixes". May I send a V2 patch series which remove these
> "Fixes" tags?
Well, if they're fixing bugs that were present in the named commit then
the the tag makes sense, it's just a question of if they are actually
for that commit or if they are fixing things for other commits like the
runtime PM enablement.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/5] spi: spi-zynqmp-gqspi: fix clk_enable/disable imbalance issue
2021-04-16 15:12 ` Mark Brown
@ 2021-04-16 15:37 ` Quanyang Wang
0 siblings, 0 replies; 10+ messages in thread
From: Quanyang Wang @ 2021-04-16 15:37 UTC (permalink / raw)
To: Mark Brown
Cc: Michal Simek, Amit Kumar Mahapatra, linux-spi, linux-arm-kernel,
linux-kernel
Hi Mark,
On 4/16/21 11:12 PM, Mark Brown wrote:
> On Fri, Apr 16, 2021 at 10:04:30PM +0800, quanyang.wang wrote:
>
>> I am sorry. These patches should NOT be with "Fixes" tag since they base on
>> the patches
>> which are not with "Fixes". May I send a V2 patch series which remove these
>> "Fixes" tags?
> Well, if they're fixing bugs that were present in the named commit then
> the the tag makes sense, it's just a question of if they are actually
> for that commit or if they are fixing things for other commits like the
> runtime PM enablement.
Yes, they are fixing bugs which are introduced by the named commit.
But if only picking they to stable without the patch "spi:
spi-zynqmp-gqspi: Fix runtime PM imbalance in zynqmp_qspi_probe",
this spi driver will not work. I don't know how to handle this
condition, so I send a V2 patch series to delete the tags.
Thanks,
Quanyang
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/5] spi: spi-zynqmp-gqspi: fix hang issue when suspend/resume
2021-04-16 0:46 [PATCH 0/5] spi: spi-zynqmp-gqspi: fix spi issues quanyang.wang
2021-04-16 0:46 ` [PATCH 1/5] spi: spi-zynqmp-gqspi: fix clk_enable/disable imbalance issue quanyang.wang
@ 2021-04-16 0:46 ` quanyang.wang
2021-04-16 0:46 ` [PATCH 3/5] spi: spi-zynqmp-gqspi: Resolved slab-out-of-bounds bug quanyang.wang
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: quanyang.wang @ 2021-04-16 0:46 UTC (permalink / raw)
To: Mark Brown, Michal Simek, Amit Kumar Mahapatra
Cc: linux-spi, linux-arm-kernel, linux-kernel, Quanyang Wang
From: Quanyang Wang <quanyang.wang@windriver.com>
After calling platform_set_drvdata(pdev, xqspi) in probe, the return
value of dev_get_drvdata(dev) is a pointer to struct zynqmp_qspi but
not struct spi_controller. A wrong structure type passing to the
functions spi_controller_suspend/resume will hang the system.
And we should check the return value of spi_controller_suspend, if
an error is returned, return it to PM subsystem to stop suspend.
Also, GQSPI_EN_MASK should be written to GQSPI_EN_OFST to enable
the spi controller in zynqmp_qspi_resume since it was disabled in
zynqmp_qspi_suspend before.
Fixes: 1c26372e5aa9 ("spi: spi-zynqmp-gqspi: Update driver to use spi-mem framework")
Signed-off-by: Quanyang Wang <quanyang.wang@windriver.com>
---
drivers/spi/spi-zynqmp-gqspi.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/spi/spi-zynqmp-gqspi.c b/drivers/spi/spi-zynqmp-gqspi.c
index f9056f0a480c..1146359528b9 100644
--- a/drivers/spi/spi-zynqmp-gqspi.c
+++ b/drivers/spi/spi-zynqmp-gqspi.c
@@ -157,6 +157,7 @@ enum mode_type {GQSPI_MODE_IO, GQSPI_MODE_DMA};
* @data_completion: completion structure
*/
struct zynqmp_qspi {
+ struct spi_controller *ctlr;
void __iomem *regs;
struct clk *refclk;
struct clk *pclk;
@@ -827,10 +828,13 @@ static void zynqmp_qspi_read_op(struct zynqmp_qspi *xqspi, u8 rx_nbits,
*/
static int __maybe_unused zynqmp_qspi_suspend(struct device *dev)
{
- struct spi_controller *ctlr = dev_get_drvdata(dev);
- struct zynqmp_qspi *xqspi = spi_controller_get_devdata(ctlr);
+ struct zynqmp_qspi *xqspi = dev_get_drvdata(dev);
+ struct spi_controller *ctlr = xqspi->ctlr;
+ int ret;
- spi_controller_suspend(ctlr);
+ ret = spi_controller_suspend(ctlr);
+ if (ret)
+ return ret;
zynqmp_gqspi_write(xqspi, GQSPI_EN_OFST, 0x0);
@@ -848,7 +852,10 @@ static int __maybe_unused zynqmp_qspi_suspend(struct device *dev)
*/
static int __maybe_unused zynqmp_qspi_resume(struct device *dev)
{
- struct spi_controller *ctlr = dev_get_drvdata(dev);
+ struct zynqmp_qspi *xqspi = dev_get_drvdata(dev);
+ struct spi_controller *ctlr = xqspi->ctlr;
+
+ zynqmp_gqspi_write(xqspi, GQSPI_EN_OFST, GQSPI_EN_MASK);
spi_controller_resume(ctlr);
@@ -865,7 +872,7 @@ static int __maybe_unused zynqmp_qspi_resume(struct device *dev)
*/
static int __maybe_unused zynqmp_runtime_suspend(struct device *dev)
{
- struct zynqmp_qspi *xqspi = (struct zynqmp_qspi *)dev_get_drvdata(dev);
+ struct zynqmp_qspi *xqspi = dev_get_drvdata(dev);
clk_disable_unprepare(xqspi->refclk);
clk_disable_unprepare(xqspi->pclk);
@@ -883,7 +890,7 @@ static int __maybe_unused zynqmp_runtime_suspend(struct device *dev)
*/
static int __maybe_unused zynqmp_runtime_resume(struct device *dev)
{
- struct zynqmp_qspi *xqspi = (struct zynqmp_qspi *)dev_get_drvdata(dev);
+ struct zynqmp_qspi *xqspi = dev_get_drvdata(dev);
int ret;
ret = clk_prepare_enable(xqspi->pclk);
@@ -1090,6 +1097,7 @@ static int zynqmp_qspi_probe(struct platform_device *pdev)
xqspi = spi_controller_get_devdata(ctlr);
xqspi->dev = dev;
+ xqspi->ctlr = ctlr;
platform_set_drvdata(pdev, xqspi);
xqspi->regs = devm_platform_ioremap_resource(pdev, 0);
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/5] spi: spi-zynqmp-gqspi: Resolved slab-out-of-bounds bug
2021-04-16 0:46 [PATCH 0/5] spi: spi-zynqmp-gqspi: fix spi issues quanyang.wang
2021-04-16 0:46 ` [PATCH 1/5] spi: spi-zynqmp-gqspi: fix clk_enable/disable imbalance issue quanyang.wang
2021-04-16 0:46 ` [PATCH 2/5] spi: spi-zynqmp-gqspi: fix hang issue when suspend/resume quanyang.wang
@ 2021-04-16 0:46 ` quanyang.wang
2021-04-16 0:46 ` [PATCH 4/5] spi: spi-zynqmp-gqspi: fix use-after-free in zynqmp_qspi_exec_op quanyang.wang
2021-04-16 0:46 ` [PATCH 5/5] spi: spi-zynqmp-gqspi: return -ENOMEM if dma_map_single fails quanyang.wang
4 siblings, 0 replies; 10+ messages in thread
From: quanyang.wang @ 2021-04-16 0:46 UTC (permalink / raw)
To: Mark Brown, Michal Simek, Amit Kumar Mahapatra
Cc: linux-spi, linux-arm-kernel, linux-kernel, Quanyang Wang
From: Amit Kumar Mahapatra <amit.kumar-mahapatra@xilinx.com>
During a transfer the driver filled the fifo with 4bytes,
even if the data that needs to be transfer is less that 4bytes.
This resulted in slab-out-of-bounds bug in KernelAddressSanitizer.
This patch resolves slab-out-of-bounds bug by filling the fifo
with the number of bytes that needs to transferred.
Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@xilinx.com>
---
drivers/spi/spi-zynqmp-gqspi.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-zynqmp-gqspi.c b/drivers/spi/spi-zynqmp-gqspi.c
index 1146359528b9..2e2607b5dee9 100644
--- a/drivers/spi/spi-zynqmp-gqspi.c
+++ b/drivers/spi/spi-zynqmp-gqspi.c
@@ -509,17 +509,19 @@ static void zynqmp_qspi_filltxfifo(struct zynqmp_qspi *xqspi, int size)
u32 count = 0, intermediate;
while ((xqspi->bytes_to_transfer > 0) && (count < size) && (xqspi->txbuf)) {
- memcpy(&intermediate, xqspi->txbuf, 4);
- zynqmp_gqspi_write(xqspi, GQSPI_TXD_OFST, intermediate);
-
if (xqspi->bytes_to_transfer >= 4) {
+ memcpy(&intermediate, xqspi->txbuf, 4);
xqspi->txbuf += 4;
xqspi->bytes_to_transfer -= 4;
+ count += 4;
} else {
+ memcpy(&intermediate, xqspi->txbuf,
+ xqspi->bytes_to_transfer);
xqspi->txbuf += xqspi->bytes_to_transfer;
xqspi->bytes_to_transfer = 0;
+ count += xqspi->bytes_to_transfer;
}
- count++;
+ zynqmp_gqspi_write(xqspi, GQSPI_TXD_OFST, intermediate);
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/5] spi: spi-zynqmp-gqspi: fix use-after-free in zynqmp_qspi_exec_op
2021-04-16 0:46 [PATCH 0/5] spi: spi-zynqmp-gqspi: fix spi issues quanyang.wang
` (2 preceding siblings ...)
2021-04-16 0:46 ` [PATCH 3/5] spi: spi-zynqmp-gqspi: Resolved slab-out-of-bounds bug quanyang.wang
@ 2021-04-16 0:46 ` quanyang.wang
2021-04-16 0:46 ` [PATCH 5/5] spi: spi-zynqmp-gqspi: return -ENOMEM if dma_map_single fails quanyang.wang
4 siblings, 0 replies; 10+ messages in thread
From: quanyang.wang @ 2021-04-16 0:46 UTC (permalink / raw)
To: Mark Brown, Michal Simek, Amit Kumar Mahapatra
Cc: linux-spi, linux-arm-kernel, linux-kernel, Quanyang Wang
From: Quanyang Wang <quanyang.wang@windriver.com>
When handling op->addr, it is using the buffer "tmpbuf" which has been
freed. This will trigger a use-after-free KASAN warning. Let's use
temporary variables to store op->addr.val and op->cmd.opcode to fix
this issue.
Fixes: 1c26372e5aa9 ("spi: spi-zynqmp-gqspi: Update driver to use spi-mem framework")
Signed-off-by: Quanyang Wang <quanyang.wang@windriver.com>
---
drivers/spi/spi-zynqmp-gqspi.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/spi/spi-zynqmp-gqspi.c b/drivers/spi/spi-zynqmp-gqspi.c
index 2e2607b5dee9..419bc1e6358b 100644
--- a/drivers/spi/spi-zynqmp-gqspi.c
+++ b/drivers/spi/spi-zynqmp-gqspi.c
@@ -928,8 +928,9 @@ static int zynqmp_qspi_exec_op(struct spi_mem *mem,
struct zynqmp_qspi *xqspi = spi_controller_get_devdata
(mem->spi->master);
int err = 0, i;
- u8 *tmpbuf;
u32 genfifoentry = 0;
+ u16 opcode = op->cmd.opcode;
+ u64 opaddr;
dev_dbg(xqspi->dev, "cmd:%#x mode:%d.%d.%d.%d\n",
op->cmd.opcode, op->cmd.buswidth, op->addr.buswidth,
@@ -942,14 +943,8 @@ static int zynqmp_qspi_exec_op(struct spi_mem *mem,
genfifoentry |= xqspi->genfifobus;
if (op->cmd.opcode) {
- tmpbuf = kzalloc(op->cmd.nbytes, GFP_KERNEL | GFP_DMA);
- if (!tmpbuf) {
- mutex_unlock(&xqspi->op_lock);
- return -ENOMEM;
- }
- tmpbuf[0] = op->cmd.opcode;
reinit_completion(&xqspi->data_completion);
- xqspi->txbuf = tmpbuf;
+ xqspi->txbuf = &opcode;
xqspi->rxbuf = NULL;
xqspi->bytes_to_transfer = op->cmd.nbytes;
xqspi->bytes_to_receive = 0;
@@ -963,13 +958,12 @@ static int zynqmp_qspi_exec_op(struct spi_mem *mem,
if (!wait_for_completion_timeout
(&xqspi->data_completion, msecs_to_jiffies(1000))) {
err = -ETIMEDOUT;
- kfree(tmpbuf);
goto return_err;
}
- kfree(tmpbuf);
}
if (op->addr.nbytes) {
+ xqspi->txbuf = &opaddr;
for (i = 0; i < op->addr.nbytes; i++) {
*(((u8 *)xqspi->txbuf) + i) = op->addr.val >>
(8 * (op->addr.nbytes - i - 1));
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 5/5] spi: spi-zynqmp-gqspi: return -ENOMEM if dma_map_single fails
2021-04-16 0:46 [PATCH 0/5] spi: spi-zynqmp-gqspi: fix spi issues quanyang.wang
` (3 preceding siblings ...)
2021-04-16 0:46 ` [PATCH 4/5] spi: spi-zynqmp-gqspi: fix use-after-free in zynqmp_qspi_exec_op quanyang.wang
@ 2021-04-16 0:46 ` quanyang.wang
4 siblings, 0 replies; 10+ messages in thread
From: quanyang.wang @ 2021-04-16 0:46 UTC (permalink / raw)
To: Mark Brown, Michal Simek, Amit Kumar Mahapatra
Cc: linux-spi, linux-arm-kernel, linux-kernel, Quanyang Wang
From: Quanyang Wang <quanyang.wang@windriver.com>
The spi controller supports 44-bit address space on AXI in DMA mode,
so set dma_addr_t width to 44-bit to avoid using a swiotlb mapping.
In addition, if dma_map_single fails, it should return immediately
instead of continuing doing the DMA operation which bases on invalid
address.
This fixes the following crash which occurs in reading a big block
from flash:
[ 123.633577] zynqmp-qspi ff0f0000.spi: swiotlb buffer is full (sz: 4194304 bytes), total 32768 (slots), used 0 (slots)
[ 123.644230] zynqmp-qspi ff0f0000.spi: ERR:rxdma:memory not mapped
[ 123.784625] Unable to handle kernel paging request at virtual address 00000000003fffc0
[ 123.792536] Mem abort info:
[ 123.795313] ESR = 0x96000145
[ 123.798351] EC = 0x25: DABT (current EL), IL = 32 bits
[ 123.803655] SET = 0, FnV = 0
[ 123.806693] EA = 0, S1PTW = 0
[ 123.809818] Data abort info:
[ 123.812683] ISV = 0, ISS = 0x00000145
[ 123.816503] CM = 1, WnR = 1
[ 123.819455] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000805047000
[ 123.825887] [00000000003fffc0] pgd=0000000803b45003, p4d=0000000803b45003, pud=0000000000000000
[ 123.834586] Internal error: Oops: 96000145 [#1] PREEMPT SMP
Fixes: 1c26372e5aa9 ("spi: spi-zynqmp-gqspi: Update driver to use spi-mem framework")
Signed-off-by: Quanyang Wang <quanyang.wang@windriver.com>
---
drivers/spi/spi-zynqmp-gqspi.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/spi/spi-zynqmp-gqspi.c b/drivers/spi/spi-zynqmp-gqspi.c
index 419bc1e6358b..328b6559bb19 100644
--- a/drivers/spi/spi-zynqmp-gqspi.c
+++ b/drivers/spi/spi-zynqmp-gqspi.c
@@ -733,7 +733,7 @@ static irqreturn_t zynqmp_qspi_irq(int irq, void *dev_id)
* zynqmp_qspi_setuprxdma - This function sets up the RX DMA operation
* @xqspi: xqspi is a pointer to the GQSPI instance.
*/
-static void zynqmp_qspi_setuprxdma(struct zynqmp_qspi *xqspi)
+static int zynqmp_qspi_setuprxdma(struct zynqmp_qspi *xqspi)
{
u32 rx_bytes, rx_rem, config_reg;
dma_addr_t addr;
@@ -747,7 +747,7 @@ static void zynqmp_qspi_setuprxdma(struct zynqmp_qspi *xqspi)
zynqmp_gqspi_write(xqspi, GQSPI_CONFIG_OFST, config_reg);
xqspi->mode = GQSPI_MODE_IO;
xqspi->dma_rx_bytes = 0;
- return;
+ return 0;
}
rx_rem = xqspi->bytes_to_receive % 4;
@@ -755,8 +755,10 @@ static void zynqmp_qspi_setuprxdma(struct zynqmp_qspi *xqspi)
addr = dma_map_single(xqspi->dev, (void *)xqspi->rxbuf,
rx_bytes, DMA_FROM_DEVICE);
- if (dma_mapping_error(xqspi->dev, addr))
+ if (dma_mapping_error(xqspi->dev, addr)) {
dev_err(xqspi->dev, "ERR:rxdma:memory not mapped\n");
+ return -ENOMEM;
+ }
xqspi->dma_rx_bytes = rx_bytes;
xqspi->dma_addr = addr;
@@ -777,6 +779,8 @@ static void zynqmp_qspi_setuprxdma(struct zynqmp_qspi *xqspi)
/* Write the number of bytes to transfer */
zynqmp_gqspi_write(xqspi, GQSPI_QSPIDMA_DST_SIZE_OFST, rx_bytes);
+
+ return 0;
}
/**
@@ -813,11 +817,17 @@ static void zynqmp_qspi_write_op(struct zynqmp_qspi *xqspi, u8 tx_nbits,
* @genfifoentry: genfifoentry is pointer to the variable in which
* GENFIFO mask is returned to calling function
*/
-static void zynqmp_qspi_read_op(struct zynqmp_qspi *xqspi, u8 rx_nbits,
+static int zynqmp_qspi_read_op(struct zynqmp_qspi *xqspi, u8 rx_nbits,
u32 genfifoentry)
{
- zynqmp_qspi_setuprxdma(xqspi);
+ int ret;
+
+ ret = zynqmp_qspi_setuprxdma(xqspi);
+ if (ret)
+ return ret;
zynqmp_qspi_fillgenfifo(xqspi, rx_nbits, genfifoentry);
+
+ return 0;
}
/**
@@ -1031,8 +1041,11 @@ static int zynqmp_qspi_exec_op(struct spi_mem *mem,
xqspi->rxbuf = (u8 *)op->data.buf.in;
xqspi->bytes_to_receive = op->data.nbytes;
xqspi->bytes_to_transfer = 0;
- zynqmp_qspi_read_op(xqspi, op->data.buswidth,
+ err = zynqmp_qspi_read_op(xqspi, op->data.buswidth,
genfifoentry);
+ if (err)
+ goto return_err;
+
zynqmp_gqspi_write(xqspi, GQSPI_CONFIG_OFST,
zynqmp_gqspi_read
(xqspi, GQSPI_CONFIG_OFST) |
@@ -1159,6 +1172,7 @@ static int zynqmp_qspi_probe(struct platform_device *pdev)
goto clk_dis_all;
}
+ dma_set_mask(&pdev->dev, DMA_BIT_MASK(44));
ctlr->bits_per_word_mask = SPI_BPW_MASK(8);
ctlr->num_chipselect = GQSPI_DEFAULT_NUM_CS;
ctlr->mem_ops = &zynqmp_qspi_mem_ops;
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread