[BUG] _regmap_raw_write() - divide-by-zero

[BUG] _regmap_raw_write() - divide-by-zero

[Russ Weight]

Hi,

I discovered a divide-by-zero bug for the SPI-Avalon use of regmap. I have
reproduced the problem in linux-next (2023-06-15). The problem was
introduced with this commit:

3981514180c9 regmap: Account for register length when chunking

I have quoted the patch below and my analysis is inline with the patch
code. I have added dmesg output including the error and stack trace at the
end of the email.

> diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
> index 7de1f27d0323..8359164bff90 100644
> --- a/drivers/base/regmap/regmap.c
> +++ b/drivers/base/regmap/regmap.c
> @@ -2064,6 +2064,8 @@ int _regmap_raw_write(struct regmap *map, unsigned int reg,
>       size_t val_count = val_len / val_bytes;
>       size_t chunk_count, chunk_bytes;
>       size_t chunk_regs = val_count;
> +     size_t max_data = map->max_raw_write - map->format.reg_bytes -
> +                     map->format.pad_bytes;

For the SPI-Avalon implementation, the values used to calculate max_data are:

regmap->max_raw_write = 4
regmap->format->reg_bytes = 4
regmap->format->pad_bytes = 0

So the above calculation for max_data is: max_data = (4 - 4 - 0) = 0

>       int ret, i;
>  
>       if (!val_count)
> @@ -2071,8 +2073,8 @@ int _regmap_raw_write(struct regmap *map, unsigned int reg,
>  
>       if (map->use_single_write)
>               chunk_regs = 1;
> -     else if (map->max_raw_write && val_len > map->max_raw_write)
> -             chunk_regs = map->max_raw_write / val_bytes;
> +     else if (map->max_raw_write && val_len > max_data)
> +             chunk_regs = max_data / val_bytes;

                chunk_regs = (0 / val_bytes) = 0
>  
>       chunk_count = val_count / chunk_regs;

  ***** chunk_count = (val_count / 0) *****


The origination of the SPI-Avalon regmap values for regmap->format->reg_bytes
and regmap->format->pad_bytes is here:

https://github.com/torvalds/linux/blob/4973ca29552864a7a047ab8a15611a585827412f/drivers/mfd/intel-m10-bmc-spi.c#L27
>       .reg_bits = 32,
        .pad_bits is not explicitly set, so it is zero

From __regmap_init:
>       map->format.reg_bytes = DIV_ROUND_UP(config->reg_bits, 8);
>       map->format.pad_bytes = config->pad_bits / 8;

The origination of the regmap->max_raw_write value is here:
https://github.com/torvalds/linux/blob/4973ca29552864a7a047ab8a15611a585827412f/drivers/base/regmap/regmap-spi-avmm.c#L663
> .max_raw_write = SPI_AVMM_VAL_SIZE * MAX_WRITE_CNT,

SPI_AVMM_VAL_SIZE is defined as 4, and MAX_WRITE_CNT is defined as 1

From __regmap_init:
>       map->max_raw_write = bus->max_raw_write;


Dmesg output:

[ 1198.244799] divide error: 0000 [#1] PREEMPT SMP PTI
[ 1198.244854] CPU: 3 PID: 896 Comm: kworker/3:2 Not tainted 6.4.0-rc6-next-20230615-regmap-broken #1
[ 1198.244910] Hardware name: Dell Inc. PowerEdge R740/0YNX56, BIOS 2.16.1 08/17/2022
[ 1198.244953] Workqueue: events_long fw_upload_main
[ 1198.244996] RIP: 0010:_regmap_raw_write+0x114/0x160
[ 1198.245036] Code: 42 48 83 c4 18 5b 5d 41 5c 41 5d 41 5e 41 5f c3 cc cc cc cc 31 d2 48 f7 f3 31 d2 48 89 c7 48 89 44 24 10 48 0f af d8 48 89 c8 <48> f7 f7 48 89 44 24 08 48 39 f9 0f 83 39 ff ff ff 41 0f b6 c0 89
[ 1198.245132] RSP: 0018:ffffb39903437d38 EFLAGS: 00010206
[ 1198.245167] RAX: 0000000000001000 RBX: 0000000000000000 RCX: 0000000000001000
[ 1198.245209] RDX: 0000000000000000 RSI: 0000000018000000 RDI: 0000000000000000
[ 1198.245249] RBP: 0000000000004000 R08: 0000000000000000 R09: 5408185ea0cb4615
[ 1198.245290] R10: 366555b501c84686 R11: e73e1b7216811b15 R12: 0000000018000000
[ 1198.245331] R13: ffff92ce16d58000 R14: ffff92ce16d58000 R15: ffff92cca2fa2400
[ 1198.245371] FS:  0000000000000000(0000) GS:ffff92cff7a40000(0000) knlGS:0000000000000000
[ 1198.245418] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1198.245452] CR2: 00005638bf126000 CR3: 0000000109600004 CR4: 00000000007706e0
[ 1198.245493] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 1198.245533] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 1198.245573] PKRU: 55555554
[ 1198.245593] Call Trace:
[ 1198.245613]  <TASK>
[ 1198.245632]  ? die+0x32/0x80
[ 1198.245665]  ? do_trap+0xd6/0x100
[ 1198.245695]  ? _regmap_raw_write+0x114/0x160
[ 1198.245731]  ? do_error_trap+0x6a/0x90
[ 1198.245759]  ? _regmap_raw_write+0x114/0x160
[ 1198.245792]  ? exc_divide_error+0x34/0x50
[ 1198.245822]  ? _regmap_raw_write+0x114/0x160
[ 1198.245853]  ? asm_exc_divide_error+0x16/0x20
[ 1198.245896]  ? _regmap_raw_write+0x114/0x160
[ 1198.245929]  ? __kmalloc_large_node+0xa5/0x110
[ 1198.245966]  regmap_raw_write+0x6c/0x90
[ 1198.245998]  regmap_bulk_write+0xb1/0x250
[ 1198.246036]  m10bmc_sec_fw_write+0x151/0x1c0 [intel_m10_bmc_sec_update]
[ 1198.246096]  fw_upload_main+0xbe/0x1f0
[ 1198.246130]  process_one_work+0x1df/0x3f0
[ 1198.246165]  worker_thread+0x4d/0x390
[ 1198.246197]  ? __pfx_worker_thread+0x10/0x10
[ 1198.246230]  kthread+0xfa/0x130
[ 1198.247352]  ? __pfx_kthread+0x10/0x10
[ 1198.248437]  ret_from_fork+0x29/0x50
[ 1198.249516]  </TASK>
[ 1198.250555] Modules linked in: xt_CHECKSUM xt_MASQUERADE xt_conntrack ipt_REJECT nf_nat_tftp nf_conntrack_tftp bridge stp llc nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat ip6table_nat ip6table_mangle ip6table_raw ip6table_security iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 iptable_mangle iptable_raw iptable_security rfkill ip_set nf_tables nfnetlink ip6table_filter iptable_filter qrtr sunrpc intel_rapl_msr intel_rapl_common intel_uncore_frequency intel_uncore_frequency_common isst_if_common skx_edac nfit libnvdimm x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel binfmt_misc intel_m10_bmc_hwmon intel_m10_bmc_sec_update vfat fat irdma intel_m10_bmc_spi ipmi_ssif regmap_spi_avmm kvm intel_m10_bmc_core ice spi_altera_platform spi_altera_dfl uio_dfl spi_altera_core irqbypass dfl_emif uio dfl_n3000_nios dfl_fme_br dfl_fme_mgr dfl_fme_region iTCO_wdt rapl dfl_fme dfl_afu
intel_pmc_bxt iTCO_vendor_support dfl_pci
[ 1198.250744]  dell_smbios intel_cstate mei_me dfl gnss ib_uverbs joydev dcdbas intel_uncore dell_wmi_descriptor wmi_bmof fpga_region acpi_ipmi pcspkr fpga_bridge ib_core i2c_i801 mei lpc_ich fpga_mgr intel_pch_thermal ipmi_si i2c_smbus ipmi_devintf ipmi_msghandler acpi_power_meter loop zram xfs i40e crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel mgag200 sha512_ssse3 tg3 wmi i2c_algo_bit scsi_dh_rdac scsi_dh_emc scsi_dh_alua ip6_tables ip_tables pkcs8_key_parser dm_multipath fuse
[ 1198.265252] ---[ end trace 0000000000000000 ]---
[ 1198.273368] pstore: backend (erst) writing error (-28)
[ 1198.273859] RIP: 0010:_regmap_raw_write+0x114/0x160
[ 1198.274337] Code: 42 48 83 c4 18 5b 5d 41 5c 41 5d 41 5e 41 5f c3 cc cc cc cc 31 d2 48 f7 f3 31 d2 48 89 c7 48 89 44 24 10 48 0f af d8 48 89 c8 <48> f7 f7 48 89 44 24 08 48 39 f9 0f 83 39 ff ff ff 41 0f b6 c0 89
[ 1198.275329] RSP: 0018:ffffb39903437d38 EFLAGS: 00010206
[ 1198.275833] RAX: 0000000000001000 RBX: 0000000000000000 RCX: 0000000000001000
[ 1198.276330] RDX: 0000000000000000 RSI: 0000000018000000 RDI: 0000000000000000
[ 1198.276827] RBP: 0000000000004000 R08: 0000000000000000 R09: 5408185ea0cb4615
[ 1198.277317] R10: 366555b501c84686 R11: e73e1b7216811b15 R12: 0000000018000000
[ 1198.277818] R13: ffff92ce16d58000 R14: ffff92ce16d58000 R15: ffff92cca2fa2400
[ 1198.278313] FS:  0000000000000000(0000) GS:ffff92cff7a40000(0000) knlGS:0000000000000000
[ 1198.278824] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1198.279332] CR2: 00005638bf126000 CR3: 0000000109600004 CR4: 00000000007706e0
[ 1198.279855] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 1198.280373] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 1198.280900] PKRU: 55555554

[ 1198.250555] Modules linked in: xt_CHECKSUM xt_MASQUERADE xt_conntrack ipt_REJECT nf_nat_tftp nf_conntrack_tftp bridge stp llc nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat ip6table_nat ip6table_mangle ip6table_raw ip6table_security iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 iptable_mangle iptable_raw iptable_security rfkill ip_set nf_tables nfnetlink ip6table_filter iptable_filter qrtr sunrpc intel_rapl_msr intel_rapl_common intel_uncore_frequency intel_uncore_frequency_common isst_if_common skx_edac nfit libnvdimm x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel binfmt_misc intel_m10_bmc_hwmon intel_m10_bmc_sec_update vfat fat irdma intel_m10_bmc_spi ipmi_ssif regmap_spi_avmm kvm intel_m10_bmc_core ice spi_altera_platform spi_altera_dfl uio_dfl spi_altera_core irqbypass dfl_emif uio dfl_n3000_nios dfl_fme_br dfl_fme_mgr dfl_fme_region iTCO_wdt rapl dfl_fme dfl_afu
intel_pmc_bxt iTCO_vendor_support dfl_pci
[ 1198.250744]  dell_smbios intel_cstate mei_me dfl gnss ib_uverbs joydev dcdbas intel_uncore dell_wmi_descriptor wmi_bmof fpga_region acpi_ipmi pcspkr fpga_bridge ib_core i2c_i801 mei lpc_ich fpga_mgr intel_pch_thermal ipmi_si i2c_smbus ipmi_devintf ipmi_msghandler acpi_power_meter loop zram xfs i40e crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel mgag200 sha512_ssse3 tg3 wmi i2c_algo_bit scsi_dh_rdac scsi_dh_emc scsi_dh_alua ip6_tables ip_tables pkcs8_key_parser dm_multipath fuse
[ 1198.265252] ---[ end trace 0000000000000000 ]---
[ 1198.273368] pstore: backend (erst) writing error (-28)
[ 1198.273859] RIP: 0010:_regmap_raw_write+0x114/0x160
[ 1198.274337] Code: 42 48 83 c4 18 5b 5d 41 5c 41 5d 41 5e 41 5f c3 cc cc cc cc 31 d2 48 f7 f3 31 d2 48 89 c7 48 89 44 24 10 48 0f af d8 48 89 c8 <48> f7 f7 48 89 44 24 08 48 39 f9 0f 83 39 ff ff ff 41 0f b6 c0 89
[ 1198.275329] RSP: 0018:ffffb39903437d38 EFLAGS: 00010206
[ 1198.275833] RAX: 0000000000001000 RBX: 0000000000000000 RCX: 0000000000001000
[ 1198.276330] RDX: 0000000000000000 RSI: 0000000018000000 RDI: 0000000000000000
[ 1198.276827] RBP: 0000000000004000 R08: 0000000000000000 R09: 5408185ea0cb4615
[ 1198.277317] R10: 366555b501c84686 R11: e73e1b7216811b15 R12: 0000000018000000
[ 1198.277818] R13: ffff92ce16d58000 R14: ffff92ce16d58000 R15: ffff92cca2fa2400
[ 1198.278313] FS:  0000000000000000(0000) GS:ffff92cff7a40000(0000) knlGS:0000000000000000
[ 1198.278824] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1198.279332] CR2: 00005638bf126000 CR3: 0000000109600004 CR4: 00000000007706e0
[ 1198.279855] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 1198.280373] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 1198.280900] PKRU: 55555554

I am willing to test any suggested changes and submit a patch, but it isn't
clear to me at this point what the fix should be.

Thanks,
- Russ

Re: [BUG] _regmap_raw_write() - divide-by-zero

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 604 bytes --]

On Fri, Jun 16, 2023 at 02:58:24PM -0700, Russ Weight wrote:

> > +     size_t max_data = map->max_raw_write - map->format.reg_bytes -
> > +                     map->format.pad_bytes;

> For the SPI-Avalon implementation, the values used to calculate max_data are:

> regmap->max_raw_write = 4
> regmap->format->reg_bytes = 4
> regmap->format->pad_bytes = 0
> 
> So the above calculation for max_data is: max_data = (4 - 4 - 0) = 0

How does this bus work?  The device can accept a maximum of 4 bytes of
data but the register is 4 bytes in length so there's no space to
transmit a value.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [BUG] _regmap_raw_write() - divide-by-zero

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 581 bytes --]

On Fri, Jun 16, 2023 at 02:58:24PM -0700, Russ Weight wrote:

> The origination of the regmap->max_raw_write value is here:
> https://github.com/torvalds/linux/blob/4973ca29552864a7a047ab8a15611a585827412f/drivers/base/regmap/regmap-spi-avmm.c#L663
> > .max_raw_write = SPI_AVMM_VAL_SIZE * MAX_WRITE_CNT,

> SPI_AVMM_VAL_SIZE is defined as 4, and MAX_WRITE_CNT is defined as 1

This should add in some headroom for the value too.

We should also fix the max_raw_write comments since it's a bit confusing
between users and buses, though AFAICT nothing outside the core ever
checks.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [BUG] _regmap_raw_write() - divide-by-zero

[Jim Wylder]

Sorry for causing the grief.

For the description would it be something like "Max transfer size
supported on a write"?
Or does that still not describe it correctly?

Jim

On Fri, Jun 16, 2023 at 5:14 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, Jun 16, 2023 at 02:58:24PM -0700, Russ Weight wrote:
>
> > The origination of the regmap->max_raw_write value is here:
> > https://github.com/torvalds/linux/blob/4973ca29552864a7a047ab8a15611a585827412f/drivers/base/regmap/regmap-spi-avmm.c#L663
> > > .max_raw_write = SPI_AVMM_VAL_SIZE * MAX_WRITE_CNT,
>
> > SPI_AVMM_VAL_SIZE is defined as 4, and MAX_WRITE_CNT is defined as 1
>
> This should add in some headroom for the value too.
>
> We should also fix the max_raw_write comments since it's a bit confusing
> between users and buses, though AFAICT nothing outside the core ever
> checks.

Re: [BUG] _regmap_raw_write() - divide-by-zero

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 586 bytes --]

On Fri, Jun 16, 2023 at 05:39:05PM -0500, Jim Wylder wrote:

Please don't top post, reply in line with needed context.  This allows
readers to readily follow the flow of conversation and understand what
you are talking about and also helps ensure that everything in the
discussion is being addressed.

> Sorry for causing the grief.
> 
> For the description would it be something like "Max transfer size
> supported on a write"?
> Or does that still not describe it correctly?

I'd probably specifically say it's the physical size but otherwise yes,
something like that.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [BUG] _regmap_raw_write() - divide-by-zero

[Russ Weight]

On 6/16/23 15:14, Mark Brown wrote:
> On Fri, Jun 16, 2023 at 02:58:24PM -0700, Russ Weight wrote:
>
>> The origination of the regmap->max_raw_write value is here:
>> https://github.com/torvalds/linux/blob/4973ca29552864a7a047ab8a15611a585827412f/drivers/base/regmap/regmap-spi-avmm.c#L663
>>> .max_raw_write = SPI_AVMM_VAL_SIZE * MAX_WRITE_CNT,
>> SPI_AVMM_VAL_SIZE is defined as 4, and MAX_WRITE_CNT is defined as 1
> This should add in some headroom for the value too.

Thanks for the guidance, Mark. We'll put a patch together to fix up the config.

- Russ

>
> We should also fix the max_raw_write comments since it's a bit confusing
> between users and buses, though AFAICT nothing outside the core ever
> checks.