[PATCH] spi: spi-mem: Add fix to avoid divide error

[PATCH] spi: spi-mem: Add fix to avoid divide error

[Raju Rangoju]

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().

Add changes to skip the 'ncylcles' calculation for zero dummy bytes.

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
 RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffb6638416b3f0
 RDX: 0000000000000000 RSI: 0000000000000018 RDI: 0000000000000000
 RBP: ffffb6638416b460 R08: 0000000000000000 R09: 0000000000000000
 R10: 0000000000000000 R11: 0000000000000000 R12: ffff9d98d476b828
 R13: 0000000000000000 R14: 0000000000000040 R15: ffffffffc0f5a3b0
 FS:  00007ed599a0dc40(0000) GS:ffff9d9c25180000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 00007d798ce9cff0 CR3: 0000000111506000 CR4: 0000000000750ef0
 PKRU: 55555554
 Call Trace:
  <TASK>
  ? show_regs+0x71/0x90
  ? die+0x38/0xa0
  ? do_trap+0xdb/0x100
  ? do_error_trap+0x75/0xb0
  ? spi_mem_calc_op_duration+0x56/0xb0
  ? exc_divide_error+0x3b/0x70
  ? spi_mem_calc_op_duration+0x56/0xb0
  ? asm_exc_divide_error+0x1b/0x20
  ? spi_mem_calc_op_duration+0x56/0xb0
  ? spinand_select_op_variant+0xee/0x190 [spinand]
  spinand_match_and_init+0x13e/0x1a0 [spinand]
  spinand_manufacturer_match+0x6e/0xa0 [spinand]
  spinand_probe+0x357/0x7f0 [spinand]
  ? kernfs_activate+0x87/0xd0
  spi_mem_probe+0x7a/0xb0
  spi_probe+0x7d/0x130
  ? driver_sysfs_add+0x66/0xd0
  really_probe+0xf7/0x3b0
  __driver_probe_device+0x8a/0x180
  driver_probe_device+0x23/0xd0
  __device_attach_driver+0xc5/0x160
  ? __pfx___device_attach_driver+0x10/0x10
  bus_for_each_drv+0x89/0xf0
  __device_attach+0xc1/0x200
  device_initial_probe+0x13/0x20
  bus_probe_device+0x9f/0xb0
  device_add+0x64f/0x870
  __spi_add_device+0x187/0x390
  spi_new_device+0x13a/0x1f0
  spinand_drv_init+0xe4/0xff0 [spinand]
  ? __pfx_spinand_drv_init+0x10/0x10 [spinand]
  do_one_initcall+0x49/0x330
  do_init_module+0x6a/0x290
  load_module+0x2522/0x2620
  init_module_from_file+0x9c/0xf0
  ? init_module_from_file+0x9c/0xf0
  idempotent_init_module+0x178/0x270
  __x64_sys_finit_module+0x77/0x100
  x64_sys_call+0x1f0b/0x26f0
  do_syscall_64+0x70/0x130
  ? srso_alias_return_thunk+0x5/0xfbef5
  ? mmap_region+0x67/0xe0
  ? srso_alias_return_thunk+0x5/0xfbef5
  ? do_mmap+0x52b/0x650
  ? srso_alias_return_thunk+0x5/0xfbef5
  ? vm_mmap_pgoff+0x152/0x200
  ? srso_alias_return_thunk+0x5/0xfbef5
  ? ksys_mmap_pgoff+0x191/0x250
  ? srso_alias_return_thunk+0x5/0xfbef5
  ? srso_alias_return_thunk+0x5/0xfbef5
  ? syscall_exit_to_user_mode+0x53/0x1c0
  ? srso_alias_return_thunk+0x5/0xfbef5
  ? do_syscall_64+0x7c/0x130
  ? srso_alias_return_thunk+0x5/0xfbef5
  ? syscall_exit_to_user_mode+0x18c/0x1c0
  ? srso_alias_return_thunk+0x5/0xfbef5
  ? do_syscall_64+0x7c/0x130
  entry_SYSCALL_64_after_hwframe+0x76/0x7e
 RIP: 0033:0x7ed59911e88d
 Code: 5b 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 73 b5 0f 00 f7 d8 64 89 01 48
 RSP: 002b:00007ffd5c54e7f8 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
 RAX: ffffffffffffffda RBX: 000060eff3d62f50 RCX: 00007ed59911e88d
 RDX: 0000000000000000 RSI: 000060efdbd38cd2 RDI: 0000000000000006
 RBP: 0000000000040000 R08: 0000000000000000 R09: 0000000000000002
 R10: 0000000000000006 R11: 0000000000000246 R12: 000060efdbd38cd2
 R13: 000060eff3d63080 R14: 000060eff3d629e0 R15: 000060eff3d63780
  </TASK>

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);
+
 	ncycles += ((op->data.nbytes * 8) / op->data.buswidth) / (op->data.dtr ? 2 : 1);
 
 	return ncycles * ns_per_cycles;
-- 
2.34.1

Re: [PATCH] spi: spi-mem: Add fix to avoid divide error

[Mark Brown]

[-- 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 --]

Re: [PATCH] spi: spi-mem: Add fix to avoid divide error

[Rangoju, Raju]

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.

Re: [PATCH] spi: spi-mem: Add fix to avoid divide error

[Mark Brown]

[-- 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 --]

Re: [PATCH] spi: spi-mem: Add fix to avoid divide error

[Miquel Raynal]

> --- 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

Re: [PATCH] spi: spi-mem: Add fix to avoid divide error

[Mark Brown]

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

Re: [PATCH] spi: spi-mem: Add fix to avoid divide error

[Dan Carpenter]

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
> 

Re: [PATCH] spi: spi-mem: Add fix to avoid divide error

[Rangoju, Raju]

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
>>

Re: [PATCH] spi: spi-mem: Add fix to avoid divide error

[Dan Carpenter]

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