* [PATCH] mtd: cqspi: Fix division by zero
@ 2021-09-14 3:21 Marek Vasut
2021-09-14 17:46 ` Pratyush Yadav
2021-10-04 13:28 ` Tom Rini
0 siblings, 2 replies; 4+ messages in thread
From: Marek Vasut @ 2021-09-14 3:21 UTC (permalink / raw)
To: u-boot; +Cc: Marek Vasut, Jagan Teki, Vignesh R, Pratyush Yadav
Both dummy.nbytes and dummy.buswidth may be zero. By not checking
the later, it is possible to trigger division by zero and a crash.
This does happen with tiny SPI NOR framework in SPL. Fix this by
adding the check and returning zero dummy bytes in such a case.
Fixes: 38b0852b0ea ("spi: cadence-qspi: Add support for octal DTR flashes")
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Jagan Teki <jagan@amarulasolutions.com>
Cc: Vignesh R <vigneshr@ti.com>
Cc: Pratyush Yadav <p.yadav@ti.com>
---
drivers/spi/cadence_qspi_apb.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index c36a652211a..429ee335db6 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -219,6 +219,9 @@ static unsigned int cadence_qspi_calc_dummy(const struct spi_mem_op *op,
{
unsigned int dummy_clk;
+ if (!op->dummy.nbytes || !op->dummy.buswidth)
+ return 0;
+
dummy_clk = op->dummy.nbytes * (8 / op->dummy.buswidth);
if (dtr)
dummy_clk /= 2;
--
2.33.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] mtd: cqspi: Fix division by zero
2021-09-14 3:21 [PATCH] mtd: cqspi: Fix division by zero Marek Vasut
@ 2021-09-14 17:46 ` Pratyush Yadav
2021-09-14 18:21 ` Marek Vasut
2021-10-04 13:28 ` Tom Rini
1 sibling, 1 reply; 4+ messages in thread
From: Pratyush Yadav @ 2021-09-14 17:46 UTC (permalink / raw)
To: Marek Vasut; +Cc: u-boot, Jagan Teki, Vignesh R
On 14/09/21 05:21AM, Marek Vasut wrote:
> Both dummy.nbytes and dummy.buswidth may be zero. By not checking
> the later, it is possible to trigger division by zero and a crash.
> This does happen with tiny SPI NOR framework in SPL. Fix this by
> adding the check and returning zero dummy bytes in such a case.
>
> Fixes: 38b0852b0ea ("spi: cadence-qspi: Add support for octal DTR flashes")
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
> Cc: Vignesh R <vigneshr@ti.com>
> Cc: Pratyush Yadav <p.yadav@ti.com>
> ---
> drivers/spi/cadence_qspi_apb.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
> index c36a652211a..429ee335db6 100644
> --- a/drivers/spi/cadence_qspi_apb.c
> +++ b/drivers/spi/cadence_qspi_apb.c
> @@ -219,6 +219,9 @@ static unsigned int cadence_qspi_calc_dummy(const struct spi_mem_op *op,
> {
> unsigned int dummy_clk;
>
> + if (!op->dummy.nbytes || !op->dummy.buswidth)
> + return 0;
Thanks. A similar fix was sent for Linux as well [0]. I don't think you
should check for dummy buswidth here since nbytes == 0 should be enough
of an indicator that the dummy phase does not exist. Any op with
dummy.nbytes > 1 and dummy.buswidth == 0 is a malformed op that should
ideally never happen, or if it does, then it should be dealt by the SPI
MEM core.
With this fixed,
Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
[0] https://patchwork.kernel.org/project/spi-devel-general/patch/92eea403-9b21-2488-9cc1-664bee760c5e@nskint.co.jp/
> +
> dummy_clk = op->dummy.nbytes * (8 / op->dummy.buswidth);
> if (dtr)
> dummy_clk /= 2;
> --
> 2.33.0
>
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mtd: cqspi: Fix division by zero
2021-09-14 17:46 ` Pratyush Yadav
@ 2021-09-14 18:21 ` Marek Vasut
0 siblings, 0 replies; 4+ messages in thread
From: Marek Vasut @ 2021-09-14 18:21 UTC (permalink / raw)
To: Pratyush Yadav; +Cc: u-boot, Jagan Teki, Vignesh R
On 9/14/21 7:46 PM, Pratyush Yadav wrote:
> On 14/09/21 05:21AM, Marek Vasut wrote:
>> Both dummy.nbytes and dummy.buswidth may be zero. By not checking
>> the later, it is possible to trigger division by zero and a crash.
>> This does happen with tiny SPI NOR framework in SPL. Fix this by
>> adding the check and returning zero dummy bytes in such a case.
>>
>> Fixes: 38b0852b0ea ("spi: cadence-qspi: Add support for octal DTR flashes")
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Jagan Teki <jagan@amarulasolutions.com>
>> Cc: Vignesh R <vigneshr@ti.com>
>> Cc: Pratyush Yadav <p.yadav@ti.com>
>> ---
>> drivers/spi/cadence_qspi_apb.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
>> index c36a652211a..429ee335db6 100644
>> --- a/drivers/spi/cadence_qspi_apb.c
>> +++ b/drivers/spi/cadence_qspi_apb.c
>> @@ -219,6 +219,9 @@ static unsigned int cadence_qspi_calc_dummy(const struct spi_mem_op *op,
>> {
>> unsigned int dummy_clk;
>>
>> + if (!op->dummy.nbytes || !op->dummy.buswidth)
>> + return 0;
>
> Thanks. A similar fix was sent for Linux as well [0]. I don't think you
> should check for dummy buswidth here since nbytes == 0 should be enough
> of an indicator that the dummy phase does not exist. Any op with
> dummy.nbytes > 1 and dummy.buswidth == 0 is a malformed op that should
> ideally never happen, or if it does, then it should be dealt by the SPI
> MEM core.
>
> With this fixed,
>
> Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
>
> [0] https://patchwork.kernel.org/project/spi-devel-general/patch/92eea403-9b21-2488-9cc1-664bee760c5e@nskint.co.jp/
>
>> +
>> dummy_clk = op->dummy.nbytes * (8 / op->dummy.buswidth);
Yes, indeed, the division by zero happens if op->dummy.buswidth == 0, so
we must check it. Checking for op->dummy.nbytes == 0 is a minor
optimization.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mtd: cqspi: Fix division by zero
2021-09-14 3:21 [PATCH] mtd: cqspi: Fix division by zero Marek Vasut
2021-09-14 17:46 ` Pratyush Yadav
@ 2021-10-04 13:28 ` Tom Rini
1 sibling, 0 replies; 4+ messages in thread
From: Tom Rini @ 2021-10-04 13:28 UTC (permalink / raw)
To: Marek Vasut; +Cc: u-boot, Jagan Teki, Vignesh R, Pratyush Yadav
[-- Attachment #1: Type: text/plain, Size: 637 bytes --]
On Tue, Sep 14, 2021 at 05:21:48AM +0200, Marek Vasut wrote:
> Both dummy.nbytes and dummy.buswidth may be zero. By not checking
> the later, it is possible to trigger division by zero and a crash.
> This does happen with tiny SPI NOR framework in SPL. Fix this by
> adding the check and returning zero dummy bytes in such a case.
>
> Fixes: 38b0852b0ea ("spi: cadence-qspi: Add support for octal DTR flashes")
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
> Cc: Vignesh R <vigneshr@ti.com>
> Cc: Pratyush Yadav <p.yadav@ti.com>
Applied to u-boot/master, thanks!
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-10-04 13:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-14 3:21 [PATCH] mtd: cqspi: Fix division by zero Marek Vasut
2021-09-14 17:46 ` Pratyush Yadav
2021-09-14 18:21 ` Marek Vasut
2021-10-04 13:28 ` Tom Rini
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).