spi: spi-mem: Add fix to avoid divide error

PATCH In review Raju Rangoju 4 participants 9 messages Apr 24, 2025 – Jun 10, 2025 SPI

Share & tools

Raju Rangoju

author

2025-04-24 12:14 UTC #0 lore raw

PATCH 1/1 spi: spi-mem: Add fix to avoid divide error

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

Co-developed-by Akshata MukundShetty <[email protected]>

Signed-off-by Akshata MukundShetty <[email protected]>

Signed-off-by Raju Rangoju <[email protected]>

---
 drivers/spi/spi-mem.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

drivers/spi/spi-mem.c +5 -1

index a31a1db07aa4..5db0639d3b01 100644
@@ -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;

Signature · 1 line

2.34.1

Mark Brown

maintainer

2025-04-24 12:16 UTC #1 lore raw

Quoted 9 lines · Raju Rangoju

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.

Rangoju, Raju

author

2025-04-24 12:27 UTC #2 lore raw

Quoted 12 lines · Mark Brown

Quoted 9 lines · Raju Rangoju

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

Quoted 6 lines

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.

Mark Brown

maintainer

2025-04-24 12:44 UTC #3 lore raw

Quoted 2 lines · Raju

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.

Miquel Raynal

Reviewed-by on v1 1/1

2025-04-24 14:25 UTC #4 lore raw

In reply to Raju Rangoju

Quoted 6 lines

drivers/spi/spi-mem.c +0 -0

@@ -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);

drivers/spi/spi-mem.c u64 spi_mem_calc_op_duration(struct spi_mem_op *op) · line 606

Quoted 10 lines

drivers/spi/spi-mem.c +5 -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 <[email protected]>

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

Mark Brown

maintainer

2025-04-25 16:03 UTC #5 lore raw

In reply to Raju Rangoju

Quoted 8 lines · 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().
[...]

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

Dan Carpenter

2025-06-02 09:45 UTC #6 lore raw

In reply to Raju Rangoju

Quoted 15 lines · Raju Rangoju

Fixes 226d6cb3cb79 ("spi: spi-mem: Estimate the time taken by operations")
Suggested-by Krishnamoorthi M <[email protected]>
Co-developed-by Akshata MukundShetty <[email protected]>
Signed-off-by Akshata MukundShetty <[email protected]>
Signed-off-by Raju Rangoju <[email protected]>
---
 drivers/spi/spi-mem.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/spi/spi-mem.c +0 -0
index a31a1db07aa4..5db0639d3b01 100644

drivers/spi/spi-mem.c u64 spi_mem_calc_op_duration(struct spi_mem_op *op) · line 603

Quoted 9 lines

drivers/spi/spi-mem.c +5 -1

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

Quoted 6 lines

drivers/spi/spi-mem.c +0 -0

 	ncycles += ((op->data.nbytes * 8) / op->data.buswidth) / (op->data.dtr ? 2 : 1);
 
 	return ncycles * ns_per_cycles;

Signature · 1 line

2.34.1

Rangoju, Raju

author

2025-06-09 10:41 UTC #7 lore raw

Quoted 28 lines · Dan Carpenter

Quoted 15 lines · Raju Rangoju
Fixes 226d6cb3cb79 ("spi: spi-mem: Estimate the time taken by operations")
Suggested-by Krishnamoorthi M <[email protected]>
Co-developed-by Akshata MukundShetty <[email protected]>
Signed-off-by Akshata MukundShetty <[email protected]>
Signed-off-by Raju Rangoju <[email protected]>
---
  drivers/spi/spi-mem.c | 6 +++++-
  1 file changed, 5 insertions(+), 1 deletion(-)
drivers/spi/spi-mem.c +0 -0
index a31a1db07aa4..5db0639d3b01 100644
drivers/spi/spi-mem.c u64 spi_mem_calc_op_duration(struct spi_mem_op *op) · line 603
Quoted 9 lines
drivers/spi/spi-mem.c +5 -1
@@ -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,

Quoted 8 lines

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.

Quoted 10 lines

regards,
dan carpenter
Quoted 6 lines
drivers/spi/spi-mem.c +0 -0
  	ncycles += ((op->data.nbytes * 8) / op->data.buswidth) / (op->data.dtr ? 2 : 1);
 
  	return ncycles * ns_per_cycles;
Signature · 1 line
2.34.1

Dan Carpenter

2025-06-10 08:03 UTC #8 lore raw

Quoted 38 lines · Raju

Quoted 35 lines · Dan Carpenter
Quoted 15 lines · Raju Rangoju
Fixes 226d6cb3cb79 ("spi: spi-mem: Estimate the time taken by operations")
Suggested-by Krishnamoorthi M <[email protected]>
Co-developed-by Akshata MukundShetty <[email protected]>
Signed-off-by Akshata MukundShetty <[email protected]>
Signed-off-by Raju Rangoju <[email protected]>
---
  drivers/spi/spi-mem.c | 6 +++++-
  1 file changed, 5 insertions(+), 1 deletion(-)
drivers/spi/spi-mem.c +0 -0
index a31a1db07aa4..5db0639d3b01 100644
drivers/spi/spi-mem.c u64 spi_mem_calc_op_duration(struct spi_mem_op *op) · line 603
Quoted 9 lines
drivers/spi/spi-mem.c +5 -1
@@ -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;

Quoted 9 lines

Quoted 1 line
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