* Re: [PATCH] spi: spi-mem: Add fix to avoid divide error
2025-04-24 12:13 [PATCH] spi: spi-mem: Add fix to avoid divide error Raju Rangoju
@ 2025-04-24 12:15 ` Mark Brown
2025-04-24 12:27 ` Rangoju, Raju
2025-04-24 14:25 ` Miquel Raynal
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Mark Brown @ 2025-04-24 12:15 UTC (permalink / raw)
To: Raju Rangoju
Cc: linux-spi, linux-kernel, miquel.raynal, Krishnamoorthi M,
Akshata MukundShetty
[-- Attachment #1: Type: text/plain, Size: 1018 bytes --]
On Thu, Apr 24, 2025 at 05:43:33PM +0530, Raju Rangoju wrote:
>
> Following divide error is fixed by this change:
>
> Oops: divide error: 0000 [#1] PREEMPT SMP NOPTI
> CPU: 15 UID: 0 PID: 1872 Comm: modprobe Not tainted 6.14.0-rc7-zero-day-+ #7
> Hardware name: AMD FOX/Lilac-RMB, BIOS RFE1007A_SPI2_11112024. 10/17/2024
> RIP: 0010:spi_mem_calc_op_duration+0x56/0xb0
> Code: 47 08 0f b6 7f 09 c1 e0 03 99 f7 ff 0f b6 51 0a 83 e2 01 8d 7a 01 99 f7 ff 0f b6 79 19 48 98 48 01 c6 0f b6 41 18 c1 e0 03 99 <f7> ff 0f b6 51 1a 83 e2 01 8d 7a 01 99 f7 ff 0f b6 79 20 31 d2 48
> RSP: 0018:ffffb6638416b3d0 EFLAGS: 00010256
Please think hard before including complete backtraces in upstream
reports, they are very large and contain almost no useful information
relative to their size so often obscure the relevant content in your
message. If part of the backtrace is usefully illustrative (it often is
for search engines if nothing else) then it's usually better to pull out
the relevant sections.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] spi: spi-mem: Add fix to avoid divide error
2025-04-24 12:15 ` Mark Brown
@ 2025-04-24 12:27 ` Rangoju, Raju
2025-04-24 12:44 ` Mark Brown
0 siblings, 1 reply; 9+ messages in thread
From: Rangoju, Raju @ 2025-04-24 12:27 UTC (permalink / raw)
To: Mark Brown
Cc: linux-spi, linux-kernel, miquel.raynal, Krishnamoorthi M,
Akshata MukundShetty
On 4/24/2025 5:45 PM, Mark Brown wrote:
> On Thu, Apr 24, 2025 at 05:43:33PM +0530, Raju Rangoju wrote:
>
>>
>> Following divide error is fixed by this change:
>>
>> Oops: divide error: 0000 [#1] PREEMPT SMP NOPTI
>> CPU: 15 UID: 0 PID: 1872 Comm: modprobe Not tainted 6.14.0-rc7-zero-day-+ #7
>> Hardware name: AMD FOX/Lilac-RMB, BIOS RFE1007A_SPI2_11112024. 10/17/2024
>> RIP: 0010:spi_mem_calc_op_duration+0x56/0xb0
>> Code: 47 08 0f b6 7f 09 c1 e0 03 99 f7 ff 0f b6 51 0a 83 e2 01 8d 7a 01 99 f7 ff 0f b6 79 19 48 98 48 01 c6 0f b6 41 18 c1 e0 03 99 <f7> ff 0f b6 51 1a 83 e2 01 8d 7a 01 99 f7 ff 0f b6 79 20 31 d2 48
>> RSP: 0018:ffffb6638416b3d0 EFLAGS: 00010256
>
> Please think hard before including complete backtraces in upstream
> reports, they are very large and contain almost no useful information
> relative to their size so often obscure the relevant content in your
> message. If part of the backtrace is usefully illustrative (it often is
> for search engines if nothing else) then it's usually better to pull out
> the relevant sections.
Sure Mark. I'll respin V2 keeping just the relevant part of call trace
and discarding rest of it.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] spi: spi-mem: Add fix to avoid divide error
2025-04-24 12:27 ` Rangoju, Raju
@ 2025-04-24 12:44 ` Mark Brown
0 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2025-04-24 12:44 UTC (permalink / raw)
To: Rangoju, Raju
Cc: linux-spi, linux-kernel, miquel.raynal, Krishnamoorthi M,
Akshata MukundShetty
[-- Attachment #1: Type: text/plain, Size: 212 bytes --]
On Thu, Apr 24, 2025 at 05:57:38PM +0530, Rangoju, Raju wrote:
> Sure Mark. I'll respin V2 keeping just the relevant part of call trace and
> discarding rest of it.
It's fine - I cut a bunch of it out locally.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] spi: spi-mem: Add fix to avoid divide error
2025-04-24 12:13 [PATCH] spi: spi-mem: Add fix to avoid divide error Raju Rangoju
2025-04-24 12:15 ` Mark Brown
@ 2025-04-24 14:25 ` Miquel Raynal
2025-04-25 16:03 ` Mark Brown
2025-06-02 9:45 ` Dan Carpenter
3 siblings, 0 replies; 9+ messages in thread
From: Miquel Raynal @ 2025-04-24 14:25 UTC (permalink / raw)
To: Raju Rangoju
Cc: broonie, linux-spi, linux-kernel, Krishnamoorthi M,
Akshata MukundShetty
> --- a/drivers/spi/spi-mem.c
> +++ b/drivers/spi/spi-mem.c
> @@ -596,7 +596,11 @@ u64 spi_mem_calc_op_duration(struct spi_mem_op *op)
> ns_per_cycles = 1000000000 / op->max_freq;
> ncycles += ((op->cmd.nbytes * 8) / op->cmd.buswidth) / (op->cmd.dtr ? 2 : 1);
> ncycles += ((op->addr.nbytes * 8) / op->addr.buswidth) / (op->addr.dtr ? 2 : 1);
> - ncycles += ((op->dummy.nbytes * 8) / op->dummy.buswidth) / (op->dummy.dtr ? 2 : 1);
> +
> + /* Dummy bytes are optional for some SPI flash memory operations */
> + if (op->dummy.nbytes)
> + ncycles += ((op->dummy.nbytes * 8) / op->dummy.buswidth) / (op->dummy.dtr ? 2 : 1);
> +
> ncycles += ((op->data.nbytes * 8) / op->data.buswidth) / (op->data.dtr ? 2 : 1);
>
> return ncycles * ns_per_cycles;
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Actually even address bytes sometimes may be skipped (eg. status reads). But there is no
reason for using spi_mem_calc_op_duration() in this case.
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] spi: spi-mem: Add fix to avoid divide error
2025-04-24 12:13 [PATCH] spi: spi-mem: Add fix to avoid divide error Raju Rangoju
2025-04-24 12:15 ` Mark Brown
2025-04-24 14:25 ` Miquel Raynal
@ 2025-04-25 16:03 ` Mark Brown
2025-06-02 9:45 ` Dan Carpenter
3 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2025-04-25 16:03 UTC (permalink / raw)
To: Raju Rangoju
Cc: linux-spi, linux-kernel, miquel.raynal, Krishnamoorthi M,
Akshata MukundShetty
On Thu, 24 Apr 2025 17:43:33 +0530, Raju Rangoju wrote:
> For some SPI flash memory operations, dummy bytes are not mandatory. For
> example, in Winbond SPINAND flash memory devices, the `write_cache` and
> `update_cache` operation variants have zero dummy bytes. Calculating the
> duration for SPI memory operations with zero dummy bytes causes
> a divide error when `ncycles` is calculated in the
> spi_mem_calc_op_duration().
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
Thanks!
[1/1] spi: spi-mem: Add fix to avoid divide error
commit: 8e4d3d8a5e51e07bd0d6cdd81b5e4af79f796927
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] spi: spi-mem: Add fix to avoid divide error
2025-04-24 12:13 [PATCH] spi: spi-mem: Add fix to avoid divide error Raju Rangoju
` (2 preceding siblings ...)
2025-04-25 16:03 ` Mark Brown
@ 2025-06-02 9:45 ` Dan Carpenter
2025-06-09 10:40 ` Rangoju, Raju
3 siblings, 1 reply; 9+ messages in thread
From: Dan Carpenter @ 2025-06-02 9:45 UTC (permalink / raw)
To: Raju Rangoju
Cc: broonie, linux-spi, linux-kernel, miquel.raynal, Krishnamoorthi M,
Akshata MukundShetty, cve, Harshit Mogalapalli
On Thu, Apr 24, 2025 at 05:43:33PM +0530, Raju Rangoju wrote:
>
> Fixes: 226d6cb3cb79 ("spi: spi-mem: Estimate the time taken by operations")
> Suggested-by: Krishnamoorthi M <krishnamoorthi.m@amd.com>
> Co-developed-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
> Signed-off-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
> Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
> ---
> drivers/spi/spi-mem.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> index a31a1db07aa4..5db0639d3b01 100644
> --- a/drivers/spi/spi-mem.c
> +++ b/drivers/spi/spi-mem.c
> @@ -596,7 +596,11 @@ u64 spi_mem_calc_op_duration(struct spi_mem_op *op)
> ns_per_cycles = 1000000000 / op->max_freq;
> ncycles += ((op->cmd.nbytes * 8) / op->cmd.buswidth) / (op->cmd.dtr ? 2 : 1);
> ncycles += ((op->addr.nbytes * 8) / op->addr.buswidth) / (op->addr.dtr ? 2 : 1);
> - ncycles += ((op->dummy.nbytes * 8) / op->dummy.buswidth) / (op->dummy.dtr ? 2 : 1);
> +
> + /* Dummy bytes are optional for some SPI flash memory operations */
> + if (op->dummy.nbytes)
> + ncycles += ((op->dummy.nbytes * 8) / op->dummy.buswidth) / (op->dummy.dtr ? 2 : 1);
> +
Hi,
We were reviewing "CVE-2025-37896: spi: spi-mem: Add fix to avoid divide
error" which was issued for this patch, but this patch is a no-op.
If op->dummy.nbytes is zero then the original code does:
ncycles += 0;
We don't divide by op->dummy.nbytes. Was something else intended?
regards,
dan carpenter
> ncycles += ((op->data.nbytes * 8) / op->data.buswidth) / (op->data.dtr ? 2 : 1);
>
> return ncycles * ns_per_cycles;
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] spi: spi-mem: Add fix to avoid divide error
2025-06-02 9:45 ` Dan Carpenter
@ 2025-06-09 10:40 ` Rangoju, Raju
2025-06-10 8:03 ` Dan Carpenter
0 siblings, 1 reply; 9+ messages in thread
From: Rangoju, Raju @ 2025-06-09 10:40 UTC (permalink / raw)
To: Dan Carpenter
Cc: broonie, linux-spi, linux-kernel, miquel.raynal, Krishnamoorthi M,
Akshata MukundShetty, cve, Harshit Mogalapalli
On 6/2/2025 3:15 PM, Dan Carpenter wrote:
> On Thu, Apr 24, 2025 at 05:43:33PM +0530, Raju Rangoju wrote:
>>
>> Fixes: 226d6cb3cb79 ("spi: spi-mem: Estimate the time taken by operations")
>> Suggested-by: Krishnamoorthi M <krishnamoorthi.m@amd.com>
>> Co-developed-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
>> Signed-off-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
>> Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
>> ---
>> drivers/spi/spi-mem.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
>> index a31a1db07aa4..5db0639d3b01 100644
>> --- a/drivers/spi/spi-mem.c
>> +++ b/drivers/spi/spi-mem.c
>> @@ -596,7 +596,11 @@ u64 spi_mem_calc_op_duration(struct spi_mem_op *op)
>> ns_per_cycles = 1000000000 / op->max_freq;
>> ncycles += ((op->cmd.nbytes * 8) / op->cmd.buswidth) / (op->cmd.dtr ? 2 : 1);
>> ncycles += ((op->addr.nbytes * 8) / op->addr.buswidth) / (op->addr.dtr ? 2 : 1);
>> - ncycles += ((op->dummy.nbytes * 8) / op->dummy.buswidth) / (op->dummy.dtr ? 2 : 1);
>> +
>> + /* Dummy bytes are optional for some SPI flash memory operations */
>> + if (op->dummy.nbytes)
>> + ncycles += ((op->dummy.nbytes * 8) / op->dummy.buswidth) / (op->dummy.dtr ? 2 : 1);
>> +
>
> Hi,
>
> We were reviewing "CVE-2025-37896: spi: spi-mem: Add fix to avoid divide
> error" which was issued for this patch, but this patch is a no-op.
>
> If op->dummy.nbytes is zero then the original code does:
>
> ncycles += 0;
>
> We don't divide by op->dummy.nbytes. Was something else intended?
Hi,
When there are no dummy bytes for an spi-mem operation, both
op->dummy.nbytes and op->dummy.buswidth are zero.
This can lead to a divide-by-zero error.
>
> regards,
> dan carpenter
>
>> ncycles += ((op->data.nbytes * 8) / op->data.buswidth) / (op->data.dtr ? 2 : 1);
>>
>> return ncycles * ns_per_cycles;
>> --
>> 2.34.1
>>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] spi: spi-mem: Add fix to avoid divide error
2025-06-09 10:40 ` Rangoju, Raju
@ 2025-06-10 8:03 ` Dan Carpenter
0 siblings, 0 replies; 9+ messages in thread
From: Dan Carpenter @ 2025-06-10 8:03 UTC (permalink / raw)
To: Rangoju, Raju
Cc: broonie, linux-spi, linux-kernel, miquel.raynal, Krishnamoorthi M,
Akshata MukundShetty, cve, Harshit Mogalapalli
On Mon, Jun 09, 2025 at 04:10:53PM +0530, Rangoju, Raju wrote:
>
>
> On 6/2/2025 3:15 PM, Dan Carpenter wrote:
> > On Thu, Apr 24, 2025 at 05:43:33PM +0530, Raju Rangoju wrote:
> > >
> > > Fixes: 226d6cb3cb79 ("spi: spi-mem: Estimate the time taken by operations")
> > > Suggested-by: Krishnamoorthi M <krishnamoorthi.m@amd.com>
> > > Co-developed-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
> > > Signed-off-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
> > > Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
> > > ---
> > > drivers/spi/spi-mem.c | 6 +++++-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> > > index a31a1db07aa4..5db0639d3b01 100644
> > > --- a/drivers/spi/spi-mem.c
> > > +++ b/drivers/spi/spi-mem.c
> > > @@ -596,7 +596,11 @@ u64 spi_mem_calc_op_duration(struct spi_mem_op *op)
> > > ns_per_cycles = 1000000000 / op->max_freq;
> > > ncycles += ((op->cmd.nbytes * 8) / op->cmd.buswidth) / (op->cmd.dtr ? 2 : 1);
> > > ncycles += ((op->addr.nbytes * 8) / op->addr.buswidth) / (op->addr.dtr ? 2 : 1);
> > > - ncycles += ((op->dummy.nbytes * 8) / op->dummy.buswidth) / (op->dummy.dtr ? 2 : 1);
> > > +
> > > + /* Dummy bytes are optional for some SPI flash memory operations */
> > > + if (op->dummy.nbytes)
> > > + ncycles += ((op->dummy.nbytes * 8) / op->dummy.buswidth) / (op->dummy.dtr ? 2 : 1);
> > > +
> >
> > Hi,
> >
> > We were reviewing "CVE-2025-37896: spi: spi-mem: Add fix to avoid divide
> > error" which was issued for this patch, but this patch is a no-op.
> >
> > If op->dummy.nbytes is zero then the original code does:
> >
> > ncycles += 0;
> >
> > We don't divide by op->dummy.nbytes. Was something else intended?
>
> Hi,
>
> When there are no dummy bytes for an spi-mem operation, both
> op->dummy.nbytes and op->dummy.buswidth are zero.
>
> This can lead to a divide-by-zero error.
>
Ah. Ok. I didn't realize they were connected that way. Thanks!
regards,
dan carpenter
^ permalink raw reply [flat|nested] 9+ messages in thread