* PMBus memory overflow @ 2025-04-17 15:39 Matt Corallo 2025-04-17 18:00 ` Guenter Roeck 0 siblings, 1 reply; 29+ messages in thread From: Matt Corallo @ 2025-04-17 15:39 UTC (permalink / raw) To: linux-hwmon When adding the PMBus entry for a FSP520-20RAB (actually FSP Twins Pro but it appears to be identical hardware with a provided SMBus -> USB adapter and it self-reports as an FSP520-20RAB when queried over PMBus using the old pmbus_peek.c script) with `echo pmbus 0x59 > /sys/bus/i2c/devices/i2c-3/new_device` I got the following BUG_ON (on Proxmox's 6.8.12-8-pve kernel). Its redundant and reports back fine on 0x60 and 0x59 for both of its modules (using pmbus_peek.c) and I'd already added 0x60 and it BUG'd adding the second module at 0x59. Apr 17 15:31:19 rackchill-refresh kernel: detected buffer overflow in memcpy Apr 17 15:31:19 rackchill-refresh kernel: ------------[ cut here ]------------ Apr 17 15:31:19 rackchill-refresh kernel: kernel BUG at lib/string_helpers.c:1048! Apr 17 15:31:19 rackchill-refresh kernel: invalid opcode: 0000 [#1] PREEMPT SMP NOPTI Apr 17 15:31:19 rackchill-refresh kernel: CPU: 18 PID: 387839 Comm: bash Tainted: P O 6.8.12-8-pve #1 Apr 17 15:31:19 rackchill-refresh kernel: Hardware name: Supermicro Super Server/X13SAE-F, BIOS 4.1 10/01/2024 Apr 17 15:31:19 rackchill-refresh kernel: RIP: 0010:fortify_panic+0x13/0x20 Apr 17 15:31:19 rackchill-refresh kernel: Code: cc 66 0f 1f 44 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 55 48 89 fe 48 c7 c7 b0 4e bf 90 48 89 e5 e8 4d f6 9a ff <0f> 0b 66 66 2e 0f 1f 84 00 00 00 00 00 90 90 90 90 90 90 90 90 90 Apr 17 15:31:19 rackchill-refresh kernel: RSP: 0018:ffffab56ae19f708 EFLAGS: 00010246 Apr 17 15:31:19 rackchill-refresh kernel: RAX: 0000000000000022 RBX: ffffab56ae19f766 RCX: 0000000000000000 Apr 17 15:31:19 rackchill-refresh kernel: RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 Apr 17 15:31:19 rackchill-refresh kernel: RBP: ffffab56ae19f708 R08: 0000000000000000 R09: 0000000000000000 Apr 17 15:31:19 rackchill-refresh kernel: R10: 0000000000000000 R11: 0000000000000000 R12: ffff8a76f8c98428 Apr 17 15:31:19 rackchill-refresh kernel: R13: ffff8a7481989400 R14: 000000000000009b R15: ffff8a8f6fe1a428 Apr 17 15:31:19 rackchill-refresh kernel: FS: 00007f3e871a1740(0000) GS:ffff8a937fd00000(0000) knlGS:0000000000000000 Apr 17 15:31:19 rackchill-refresh kernel: CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 Apr 17 15:31:19 rackchill-refresh kernel: CR2: 00007ff6328e0000 CR3: 00000011f6ba0002 CR4: 0000000000f72ef0 Apr 17 15:31:19 rackchill-refresh kernel: PKRU: 55555554 Apr 17 15:31:19 rackchill-refresh kernel: Call Trace: Apr 17 15:31:19 rackchill-refresh kernel: <TASK> Apr 17 15:31:19 rackchill-refresh kernel: ? show_regs+0x6d/0x80 Apr 17 15:31:19 rackchill-refresh kernel: ? die+0x37/0xa0 Apr 17 15:31:19 rackchill-refresh kernel: ? do_trap+0xd4/0xf0 Apr 17 15:31:19 rackchill-refresh kernel: ? do_error_trap+0x71/0xb0 Apr 17 15:31:19 rackchill-refresh kernel: ? fortify_panic+0x13/0x20 Apr 17 15:31:19 rackchill-refresh kernel: ? exc_invalid_op+0x52/0x80 Apr 17 15:31:19 rackchill-refresh kernel: ? fortify_panic+0x13/0x20 Apr 17 15:31:19 rackchill-refresh kernel: ? asm_exc_invalid_op+0x1b/0x20 Apr 17 15:31:19 rackchill-refresh kernel: ? fortify_panic+0x13/0x20 Apr 17 15:31:19 rackchill-refresh kernel: ? fortify_panic+0x13/0x20 Apr 17 15:31:19 rackchill-refresh kernel: i2c_smbus_read_block_data+0x116/0x120 Apr 17 15:31:19 rackchill-refresh kernel: pmbus_check_block_register.constprop.0+0x77/0x100 [pmbus_core] Apr 17 15:31:19 rackchill-refresh kernel: pmbus_do_probe+0x11a5/0x1e00 [pmbus_core] Apr 17 15:31:19 rackchill-refresh kernel: ? __kmalloc_node_track_caller+0x1c9/0x430 Apr 17 15:31:19 rackchill-refresh kernel: ? devres_add+0x6f/0xf0 Apr 17 15:31:19 rackchill-refresh kernel: pmbus_probe+0x74/0xb0 [pmbus] Apr 17 15:31:19 rackchill-refresh kernel: i2c_device_probe+0x13b/0x300 Apr 17 15:31:19 rackchill-refresh kernel: really_probe+0x1c9/0x430 Apr 17 15:31:19 rackchill-refresh kernel: __driver_probe_device+0x8c/0x190 Apr 17 15:31:19 rackchill-refresh kernel: driver_probe_device+0x24/0xd0 Apr 17 15:31:19 rackchill-refresh kernel: __device_attach_driver+0xcd/0x170 Apr 17 15:31:19 rackchill-refresh kernel: ? __pfx___device_attach_driver+0x10/0x10 Apr 17 15:31:19 rackchill-refresh kernel: bus_for_each_drv+0x94/0xf0 Apr 17 15:31:19 rackchill-refresh kernel: __device_attach+0xb6/0x1d0 Apr 17 15:31:19 rackchill-refresh kernel: device_initial_probe+0x13/0x20 Apr 17 15:31:19 rackchill-refresh kernel: bus_probe_device+0x9f/0xb0 Apr 17 15:31:19 rackchill-refresh kernel: device_add+0x6d7/0x8f0 Apr 17 15:31:19 rackchill-refresh kernel: ? hrtimer_init+0x27/0x90 Apr 17 15:31:19 rackchill-refresh kernel: device_register+0x1a/0x30 Apr 17 15:31:19 rackchill-refresh kernel: i2c_new_client_device+0x1fb/0x3f0 Apr 17 15:31:19 rackchill-refresh kernel: new_device_store+0x113/0x290 Apr 17 15:31:19 rackchill-refresh kernel: dev_attr_store+0x14/0x40 Apr 17 15:31:19 rackchill-refresh kernel: sysfs_kf_write+0x3b/0x60 Apr 17 15:31:19 rackchill-refresh kernel: kernfs_fop_write_iter+0x130/0x210 Apr 17 15:31:19 rackchill-refresh kernel: vfs_write+0x2a5/0x480 Apr 17 15:31:19 rackchill-refresh kernel: ksys_write+0x73/0x100 Apr 17 15:31:19 rackchill-refresh kernel: __x64_sys_write+0x19/0x30 Apr 17 15:31:19 rackchill-refresh kernel: x64_sys_call+0x200f/0x2480 Apr 17 15:31:19 rackchill-refresh kernel: do_syscall_64+0x81/0x170 Apr 17 15:31:19 rackchill-refresh kernel: ? __handle_mm_fault+0xba9/0xf70 Apr 17 15:31:19 rackchill-refresh kernel: ? __count_memcg_events+0x6f/0xe0 Apr 17 15:31:19 rackchill-refresh kernel: ? count_memcg_events.constprop.0+0x2a/0x50 Apr 17 15:31:19 rackchill-refresh kernel: ? handle_mm_fault+0xad/0x380 Apr 17 15:31:19 rackchill-refresh kernel: ? do_user_addr_fault+0x33e/0x660 Apr 17 15:31:19 rackchill-refresh kernel: ? irqentry_exit_to_user_mode+0x7b/0x260 Apr 17 15:31:19 rackchill-refresh kernel: ? irqentry_exit+0x43/0x50 Apr 17 15:31:19 rackchill-refresh kernel: ? exc_page_fault+0x94/0x1b0 Apr 17 15:31:19 rackchill-refresh kernel: entry_SYSCALL_64_after_hwframe+0x78/0x80 Apr 17 15:31:19 rackchill-refresh kernel: RIP: 0033:0x7f3e8729c300 Apr 17 15:31:19 rackchill-refresh kernel: Code: 40 00 48 8b 15 01 9b 0d 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 80 3d e1 22 0e 00 00 74 17 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 58 c3 0f 1f 80 00 00 00 00 48 83 ec 28 48 89 Apr 17 15:31:19 rackchill-refresh kernel: RSP: 002b:00007ffcd6171828 EFLAGS: 00000202 ORIG_RAX: 0000000000000001 Apr 17 15:31:19 rackchill-refresh kernel: RAX: ffffffffffffffda RBX: 000000000000000b RCX: 00007f3e8729c300 Apr 17 15:31:19 rackchill-refresh kernel: RDX: 000000000000000b RSI: 00005a5e6f9bb6f0 RDI: 0000000000000001 Apr 17 15:31:19 rackchill-refresh kernel: RBP: 00005a5e6f9bb6f0 R08: 0000000000000007 R09: 0000000000000073 Apr 17 15:31:19 rackchill-refresh kernel: R10: 0000000000000000 R11: 0000000000000202 R12: 000000000000000b Apr 17 15:31:19 rackchill-refresh kernel: R13: 00007f3e87377760 R14: 000000000000000b R15: 00007f3e873729e0 Apr 17 15:31:19 rackchill-refresh kernel: </TASK> Apr 17 15:31:19 rackchill-refresh kernel: Modules linked in: pmbus pmbus_core tcp_diag inet_diag ebt_arp xt_mac veth rbd ceph libceph cmac nls_utf8 cifs cifs_arc4 nls_ucs2_utils rdma_cm iw_cm ib_cm cifs_md4 netfs ebtable_filter ebtables ip6table_raw ip6t_REJECT nf_reject_ipv6 ip6table_filter ip6_tables iptable_raw ipt_REJECT nf_reject_ipv4 xt_mark xt_set xt_physdev xt_addrtype xt_comment xt_multiport xt_conntrack nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 xt_tcpudp iptable_filter ip_set_hash_net ip_set sctp ip6_udp_tunnel udp_tunnel nf_tables nvme_fabrics nvme_keyring bonding sunrpc softdog nfnetlink_log binfmt_misc nfnetlink intel_rapl_msr intel_rapl_common intel_uncore_frequency intel_uncore_frequency_common snd_sof_pci_intel_tgl snd_sof_intel_hda_common soundwire_intel snd_sof_intel_hda_mlink soundwire_cadence snd_sof_intel_hda x86_pkg_temp_thermal snd_sof_pci intel_powerclamp snd_sof_xtensa_dsp snd_sof snd_sof_utils snd_soc_hdac_hda coretemp snd_hda_ext_core snd_soc_acpi_intel_match snd_soc_acpi soundwire_generic_allocation soundwire_bus Apr 17 15:31:19 rackchill-refresh kernel: kvm_intel snd_soc_core snd_hda_codec_realtek snd_compress ac97_bus snd_hda_codec_generic snd_pcm_dmaengine snd_hda_intel kvm snd_intel_dspcfg snd_intel_sdw_acpi snd_hda_codec snd_hda_core ipmi_ssif irqbypass snd_hwdep mei_hdcp mei_pxp cmdlinepart snd_pcm spi_nor snd_timer rapl ast snd acpi_ipmi intel_cstate ucsi_acpi mtd wmi_bmof i2c_algo_bit soundcore mei_me pcspkr intel_pmc_core typec_ucsi ipmi_si mei ipmi_devintf intel_vsec typec pmt_telemetry ipmi_msghandler pmt_class acpi_pad acpi_tad joydev input_leds mac_hid zfs(PO) spl(O) vhost_net vhost vhost_iotlb tap nct6775 nct6775_core hwmon_vid efi_pstore dmi_sysfs ip_tables x_tables autofs4 btrfs blake2b_generic xor raid6_pq libcrc32c dm_crypt mlx5_ib ib_uverbs macsec ib_core rndis_host cdc_ether usbnet mii usbmouse hid_cp2112 hid_generic usbhid hid crct10dif_pclmul mlx5_core crc32_pclmul polyval_clmulni nvme xhci_pci polyval_generic ghash_clmulni_intel xhci_pci_renesas sha256_ssse3 sha1_ssse3 e1000e intel_lpss_pci mlxfw nvme_core i2c_i801 ahci igc psample Apr 17 15:31:19 rackchill-refresh kernel: intel_lpss spi_intel_pci xhci_hcd spi_intel i2c_smbus tls idma64 nvme_auth libahci pci_hyperv_intf video pinctrl_alderlake wmi aesni_intel crypto_simd cryptd Apr 17 15:31:19 rackchill-refresh kernel: ---[ end trace 0000000000000000 ]--- Apr 17 15:31:19 rackchill-refresh kernel: RIP: 0010:fortify_panic+0x13/0x20 Apr 17 15:31:19 rackchill-refresh kernel: Code: cc 66 0f 1f 44 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 55 48 89 fe 48 c7 c7 b0 4e bf 90 48 89 e5 e8 4d f6 9a ff <0f> 0b 66 66 2e 0f 1f 84 00 00 00 00 00 90 90 90 90 90 90 90 90 90 Apr 17 15:31:19 rackchill-refresh kernel: RSP: 0018:ffffab56ae19f708 EFLAGS: 00010246 Apr 17 15:31:19 rackchill-refresh kernel: RAX: 0000000000000022 RBX: ffffab56ae19f766 RCX: 0000000000000000 Apr 17 15:31:19 rackchill-refresh kernel: RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 Apr 17 15:31:19 rackchill-refresh kernel: RBP: ffffab56ae19f708 R08: 0000000000000000 R09: 0000000000000000 Apr 17 15:31:19 rackchill-refresh kernel: R10: 0000000000000000 R11: 0000000000000000 R12: ffff8a76f8c98428 Apr 17 15:31:19 rackchill-refresh kernel: R13: ffff8a7481989400 R14: 000000000000009b R15: ffff8a8f6fe1a428 Apr 17 15:31:19 rackchill-refresh kernel: FS: 00007f3e871a1740(0000) GS:ffff8a937fd00000(0000) knlGS:0000000000000000 Apr 17 15:31:19 rackchill-refresh kernel: CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 Apr 17 15:31:19 rackchill-refresh kernel: CR2: 00007ff6328e0000 CR3: 00000011f6ba0002 CR4: 0000000000f72ef0 Apr 17 15:31:19 rackchill-refresh kernel: PKRU: 55555554 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2025-04-17 15:39 PMBus memory overflow Matt Corallo @ 2025-04-17 18:00 ` Guenter Roeck 2025-04-17 18:14 ` Matt Corallo 0 siblings, 1 reply; 29+ messages in thread From: Guenter Roeck @ 2025-04-17 18:00 UTC (permalink / raw) To: Matt Corallo; +Cc: linux-hwmon On Thu, Apr 17, 2025 at 11:39:14AM -0400, Matt Corallo wrote: > When adding the PMBus entry for a FSP520-20RAB (actually FSP Twins Pro but > it appears to be identical hardware with a provided SMBus -> USB adapter and > it self-reports as an FSP520-20RAB when queried over PMBus using the old > pmbus_peek.c script) with `echo pmbus 0x59 > > /sys/bus/i2c/devices/i2c-3/new_device` I got the following BUG_ON (on > Proxmox's 6.8.12-8-pve kernel). Its redundant and reports back fine on 0x60 > and 0x59 for both of its modules (using pmbus_peek.c) and I'd already added > 0x60 and it BUG'd adding the second module at 0x59. > ... > Apr 17 15:31:19 rackchill-refresh kernel: i2c_smbus_read_block_data+0x116/0x120 > Apr 17 15:31:19 rackchill-refresh kernel: > pmbus_check_block_register.constprop.0+0x77/0x100 [pmbus_core] Interesting. That function reads into a buffer which is larger than the largest valid PMBus transaction. I can only imagine that the controller returns invalid data in that transaction, where the first returned byte is not the length of the transfer but something else. i2c_smbus_read_block_data() does an unconditional memcpy(values, &data.block[1], data.block[0]); which of course will go haywire if data.block[0] (or in other words the first data byte returned from the device) exceeds the size of the data buffer. Do you happen to have a datasheet ? Guenter ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2025-04-17 18:00 ` Guenter Roeck @ 2025-04-17 18:14 ` Matt Corallo 2025-04-18 1:21 ` Guenter Roeck 0 siblings, 1 reply; 29+ messages in thread From: Matt Corallo @ 2025-04-17 18:14 UTC (permalink / raw) To: Guenter Roeck; +Cc: linux-hwmon On 4/17/25 2:00 PM, Guenter Roeck wrote: > On Thu, Apr 17, 2025 at 11:39:14AM -0400, Matt Corallo wrote: >> When adding the PMBus entry for a FSP520-20RAB (actually FSP Twins Pro but >> it appears to be identical hardware with a provided SMBus -> USB adapter and >> it self-reports as an FSP520-20RAB when queried over PMBus using the old >> pmbus_peek.c script) with `echo pmbus 0x59 > >> /sys/bus/i2c/devices/i2c-3/new_device` I got the following BUG_ON (on >> Proxmox's 6.8.12-8-pve kernel). Its redundant and reports back fine on 0x60 >> and 0x59 for both of its modules (using pmbus_peek.c) and I'd already added >> 0x60 and it BUG'd adding the second module at 0x59. >> > ... >> Apr 17 15:31:19 rackchill-refresh kernel: i2c_smbus_read_block_data+0x116/0x120 >> Apr 17 15:31:19 rackchill-refresh kernel: >> pmbus_check_block_register.constprop.0+0x77/0x100 [pmbus_core] > > Interesting. That function reads into a buffer which is larger than the > largest valid PMBus transaction. I can only imagine that the controller > returns invalid data in that transaction, where the first returned byte > is not the length of the transfer but something else. > i2c_smbus_read_block_data() does an unconditional > memcpy(values, &data.block[1], data.block[0]); > which of course will go haywire if data.block[0] (or in other words the > first data byte returned from the device) exceeds the size of the > data buffer. Do you happen to have a datasheet ? I do not, sadly (though FSP support has been rumored to help out at least marginally, though they haven't been useful for me). Interestingly the (I guess ancient now) pmbus_peek.c script has no issues reading from it (added a quick print on the -E2BIG line and it didn't get hit). pmbuss_peek.c says the following: root@rackchill-refresh:~# ./a.out -b /dev/i2c-3 -s 0x59 PMBus slave on /dev/i2c-3, address 0x59 Inventory Data: Manufacturer: FSP-GROUP Model: FSP520-20RAB Revision: (null) Built on: Serial: IC Device: PIC24FJ32GA004 PMBus revisions (0x22): part I, ver 1.1; part II, ver 1.2 Capabilities (0x90): PEC, SMBALERT#, 100 KHz Status 0000: Attribute Values: page 00: (BITMAP) operation 00: (BITMAP) on_off_config 15: (BITMAP) vout_mode 17: (BITMAP) fan_command_1 0000: 0 ein ba4ba00938ba: 5.26211e+06 eout 1e56a12038ba: 5.29753e+06 vin f1e8: 122 Volts iin c09c: 0.609375 Amperes vout 1869: 12.2051 Volts iout caad: 5.35156 Amperes temperature_1 001c: 28 degrees Celsius temperature_2 0024: 36 degrees Celsius fan_speed_1 1a1f: 4344 frequency f0ec: 59 pout f108: 66 Watts pin f128: 74 Watts mfr_vout_min 0000: 0 Volts mfr_iout_max 0000: 0 Amperes root@rackchill-refresh:~# ./a.out -b /dev/i2c-3 -s 0x58 PMBus slave on /dev/i2c-3, address 0x58 Inventory Data: Manufacturer: FSP-GROUP Model: FSP520-20RAB Revision: (null) Built on: Serial: IC Device: PIC24FJ32GA004 PMBus revisions (0x22): part I, ver 1.1; part II, ver 1.2 Capabilities (0x90): PEC, SMBALERT#, 100 KHz Status 0000: Attribute Values: page 00: (BITMAP) operation 00: (BITMAP) on_off_config 15: (BITMAP) vout_mode 17: (BITMAP) fan_command_1 0000: 0 ein 0574e89f8070: 7.63164e+06 eout 613f88b78070: 4.47254e+06 vin f1e8: 122 Volts iin c039: 0.222656 Amperes vout 1871: 12.2207 Volts iout c11e: 1.11719 Amperes temperature_1 001b: 27 degrees Celsius temperature_2 0024: 36 degrees Celsius fan_speed_1 13e5: 3988 frequency f0ec: 59 pout f038: 14 Watts pin f050: 20 Watts mfr_vout_min 0000: 0 Volts mfr_iout_max 0000: 0 Amperes ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2025-04-17 18:14 ` Matt Corallo @ 2025-04-18 1:21 ` Guenter Roeck 2025-04-18 21:03 ` Matt Corallo 0 siblings, 1 reply; 29+ messages in thread From: Guenter Roeck @ 2025-04-18 1:21 UTC (permalink / raw) To: Matt Corallo; +Cc: linux-hwmon On 4/17/25 11:14, Matt Corallo wrote: > > > On 4/17/25 2:00 PM, Guenter Roeck wrote: >> On Thu, Apr 17, 2025 at 11:39:14AM -0400, Matt Corallo wrote: >>> When adding the PMBus entry for a FSP520-20RAB (actually FSP Twins Pro but >>> it appears to be identical hardware with a provided SMBus -> USB adapter and >>> it self-reports as an FSP520-20RAB when queried over PMBus using the old >>> pmbus_peek.c script) with `echo pmbus 0x59 > >>> /sys/bus/i2c/devices/i2c-3/new_device` I got the following BUG_ON (on >>> Proxmox's 6.8.12-8-pve kernel). Its redundant and reports back fine on 0x60 >>> and 0x59 for both of its modules (using pmbus_peek.c) and I'd already added >>> 0x60 and it BUG'd adding the second module at 0x59. >>> >> ... >>> Apr 17 15:31:19 rackchill-refresh kernel: i2c_smbus_read_block_data+0x116/0x120 >>> Apr 17 15:31:19 rackchill-refresh kernel: >>> pmbus_check_block_register.constprop.0+0x77/0x100 [pmbus_core] >> >> Interesting. That function reads into a buffer which is larger than the >> largest valid PMBus transaction. I can only imagine that the controller >> returns invalid data in that transaction, where the first returned byte >> is not the length of the transfer but something else. >> i2c_smbus_read_block_data() does an unconditional >> memcpy(values, &data.block[1], data.block[0]); >> which of course will go haywire if data.block[0] (or in other words the >> first data byte returned from the device) exceeds the size of the >> data buffer. Do you happen to have a datasheet ? > > I do not, sadly (though FSP support has been rumored to help out at least marginally, though they haven't been useful for me). Interestingly the (I guess ancient now) pmbus_peek.c script has no issues reading from it (added a quick print on the -E2BIG line and it didn't get hit). pmbuss_peek.c says the following: > > root@rackchill-refresh:~# ./a.out -b /dev/i2c-3 -s 0x59 > PMBus slave on /dev/i2c-3, address 0x59 > pmbus_peek supports reading up to 255 bytes into the receive buffer. Anything above 32 bytes violates the SMBus specification. I found that the I2C controller driver should block that. If it doesn't, all kinds of chips could trigger this problem. Do you know which I2C controller is used by that system ? You mentioned an SMBus - USB adapter. The drivers for the adapters I am aware of (Diolan, Devantech) do validate the return length, so I assume you use something else or maybe an out-of-tree driver. Please let me know the adapter you use. If the driver is not upstream, a pointer to the driver source would also be helpful. Thanks, Guenter ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2025-04-18 1:21 ` Guenter Roeck @ 2025-04-18 21:03 ` Matt Corallo 2025-04-18 22:30 ` Guenter Roeck 0 siblings, 1 reply; 29+ messages in thread From: Matt Corallo @ 2025-04-18 21:03 UTC (permalink / raw) To: Guenter Roeck; +Cc: linux-hwmon On 4/17/25 9:21 PM, Guenter Roeck wrote: > On 4/17/25 11:14, Matt Corallo wrote: >> I do not, sadly (though FSP support has been rumored to help out at least marginally, though they >> haven't been useful for me). Interestingly the (I guess ancient now) pmbus_peek.c script has no >> issues reading from it (added a quick print on the -E2BIG line and it didn't get hit). >> pmbuss_peek.c says the following: >> >> root@rackchill-refresh:~# ./a.out -b /dev/i2c-3 -s 0x59 >> PMBus slave on /dev/i2c-3, address 0x59 >> > > pmbus_peek supports reading up to 255 bytes into the receive buffer. Hmm, I'm using this version, which on L622 checks for a length > 32 (and doesn't get hit in the -s command) - https://github.com/jktjkt/pmbus_peek/blob/master/pmbus_peek.c#L622 > Anything above 32 bytes violates the SMBus specification. I found that > the I2C controller driver should block that. If it doesn't, all kinds > of chips could trigger this problem. Do you know which I2C controller > is used by that system ? You mentioned an SMBus - USB adapter. The drivers > for the adapters I am aware of (Diolan, Devantech) do validate the return > length, so I assume you use something else or maybe an out-of-tree driver. The driver appears to be in-tree (or at least was auto-loaded by the Proxmox kernel, which is mostly just the Ubuntu kernel). Its a 10c4:ea90 Silicon Labs CP2112 HID I2C Bridge. Still, more generally, presumably it shouldn't be the case that a defective USB device can cause the kernel to memcpy past a buffer? I guess for hardened kernels it gets caught (though once the process gets killed by the hardening the system is still somewhat shaky, reboots dont work etc), but is there a build option that would turn this into a security vuln? Matt ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2025-04-18 21:03 ` Matt Corallo @ 2025-04-18 22:30 ` Guenter Roeck 2025-04-19 17:53 ` Matt Corallo 0 siblings, 1 reply; 29+ messages in thread From: Guenter Roeck @ 2025-04-18 22:30 UTC (permalink / raw) To: Matt Corallo; +Cc: linux-hwmon On 4/18/25 14:03, Matt Corallo wrote: > > > On 4/17/25 9:21 PM, Guenter Roeck wrote: >> On 4/17/25 11:14, Matt Corallo wrote: >>> I do not, sadly (though FSP support has been rumored to help out at least marginally, though they haven't been useful for me). Interestingly the (I guess ancient now) pmbus_peek.c script has no issues reading from it (added a quick print on the -E2BIG line and it didn't get hit). pmbuss_peek.c says the following: >>> >>> root@rackchill-refresh:~# ./a.out -b /dev/i2c-3 -s 0x59 >>> PMBus slave on /dev/i2c-3, address 0x59 >>> >> >> pmbus_peek supports reading up to 255 bytes into the receive buffer. > > Hmm, I'm using this version, which on L622 checks for a length > 32 (and doesn't get hit in the -s command) - https://github.com/jktjkt/pmbus_peek/blob/master/pmbus_peek.c#L622 > Sorry, I am only guessing here. The code in pmbus_peek.c is a bit odd, though: It does validate the first byte, but then it jumps to try_i2c: and tries again, this time reading up to 255 bytes. I am not really sure if that ends up reporting an error or not. The comment "NOTE: this probably won't be visible" doesn't really improve my confidence that it will report the problem. >> Anything above 32 bytes violates the SMBus specification. I found that >> the I2C controller driver should block that. If it doesn't, all kinds >> of chips could trigger this problem. Do you know which I2C controller >> is used by that system ? You mentioned an SMBus - USB adapter. The drivers >> for the adapters I am aware of (Diolan, Devantech) do validate the return >> length, so I assume you use something else or maybe an out-of-tree driver. > > The driver appears to be in-tree (or at least was auto-loaded by the Proxmox kernel, which is mostly just the Ubuntu kernel). Its a 10c4:ea90 Silicon Labs CP2112 HID I2C Bridge. > So that is drivers/hid/hid-cp2112.c. Unless I am missing something, that driver does not validate the contents of the first returned data byte when executing an I2C_SMBUS_BLOCK_DATA read command. > Still, more generally, presumably it shouldn't be the case that a defective USB device can cause the kernel to memcpy past a buffer? I guess for hardened kernels it gets caught (though once the process gets killed by the hardening the system is still somewhat shaky, reboots dont work etc), but is there a build option that would turn this into a security vuln? > Exactly my point. A defective USB device should most definitely _not_ cause the kernel to memcpy past the end of a buffer. Of course, question is if i2c_smbus_read_block_data() should blindly copy more than 32 bytes just because the first data byte returned from the device says so, but that is a question to ask the i2c maintainer after we figure out what is going on. If you are up to compiling code, it would be great if you could add some debug logs into pmbus_peek.c and/or into the hid-cp2112.c driver to see what is actually returned from the chip. Thanks, Guenter ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2025-04-18 22:30 ` Guenter Roeck @ 2025-04-19 17:53 ` Matt Corallo 2025-04-19 19:05 ` Guenter Roeck 0 siblings, 1 reply; 29+ messages in thread From: Matt Corallo @ 2025-04-19 17:53 UTC (permalink / raw) To: Guenter Roeck; +Cc: linux-hwmon On 4/18/25 6:30 PM, Guenter Roeck wrote: > On 4/18/25 14:03, Matt Corallo wrote: >> >> >> On 4/17/25 9:21 PM, Guenter Roeck wrote: >>> On 4/17/25 11:14, Matt Corallo wrote: >>>> I do not, sadly (though FSP support has been rumored to help out at least marginally, though >>>> they haven't been useful for me). Interestingly the (I guess ancient now) pmbus_peek.c script >>>> has no issues reading from it (added a quick print on the -E2BIG line and it didn't get hit). >>>> pmbuss_peek.c says the following: >>>> >>>> root@rackchill-refresh:~# ./a.out -b /dev/i2c-3 -s 0x59 >>>> PMBus slave on /dev/i2c-3, address 0x59 >>>> >>> >>> pmbus_peek supports reading up to 255 bytes into the receive buffer. >> >> Hmm, I'm using this version, which on L622 checks for a length > 32 (and doesn't get hit in the -s >> command) - https://github.com/jktjkt/pmbus_peek/blob/master/pmbus_peek.c#L622 >> > > Sorry, I am only guessing here. The code in pmbus_peek.c is a bit odd, though: > It does validate the first byte, but then it jumps to try_i2c: and tries again, > this time reading up to 255 bytes. I am not really sure if that ends up reporting > an error or not. The comment "NOTE: this probably won't be visible" doesn't > really improve my confidence that it will report the problem. Sure, that's why I `printf("BAD LEN\n");`'d there but didn't see it get hit. > If you are up to compiling code, it would be great if you could add some debug logs > into pmbus_peek.c and/or into the hid-cp2112.c driver to see what is actually > returned from the chip. Sure, of course. Obviously easier to do in pmbus_peek if you have any suggested logs you want. The device caps output is (and still never hit my BAD LEN printf): ./a.out -b /dev/i2c-3 -l -v 0x58 PMBus slave on /dev/i2c-3, address 0x58 Inventory Data: Manufacturer: FSP-GROUP Model: FSP520-20RAB Revision: (null) Built on: Serial: IC Device: PIC24FJ32GA004 PMBus revisions (0x22): part I, ver 1.1; part II, ver 1.2 Capabilities (0x90): PEC, SMBALERT#, 100 KHz Supported Commands: 00 page rw u8 (bitmask) 01 operation rw u8 (bitmask) 02 on_off_config rw u8 (bitmask) 03 clear_fault w nodata 05 page_plus_read w block 19 capability r u8 (bitmask) 1a query rw process_call 1b smbalert_mask rw block 20 vout_mode r u8 (bitmask) 30 coefficients r process_call 3b fan_command_1 rw s16 (LINEAR) 79 status_word r u16 (bitmask) 7a status_vout r u8 (bitmask) 7b status_iout r u8 (bitmask) 7c status_input r u8 (bitmask) 7d status_temperature r u8 (bitmask) 81 status_fans_1_2 rw u8 (bitmask) 86 read_ein r block(6), Energy counter (DIRECT) 87 read_eout r block(6), Energy counter (DIRECT) 88 read_vin r s16 (LINEAR), Volts 89 read_iin r s16 (LINEAR), Amperes 8b read_vout r x16 (VOUT_MODE), Volts 8c read_iout r s16 (LINEAR), Amperes 8d read_temperature_1 r s16 (LINEAR), degrees Celsius 8e read_temperature_2 r s16 (LINEAR), degrees Celsius 90 read_fan_speed_1 r s16 (LINEAR) 95 read_frequency r s16 (LINEAR) 96 read_pout r s16 (LINEAR), Watts 97 read_pin r s16 (LINEAR), Watts 98 pmbus_revision r u8 (bitmask) 99 mfr_id r block, ISO 8859/1 string 9a mfr_model r block, ISO 8859/1 string 9d mfr_date r block, ISO 8859/1 string 9e mfr_serial r block, ISO 8859/1 string a4 mfr_vout_min r s16 (LINEAR), Volts a5 mfr_iout_max r s16 (LINEAR), Amperes ad ic_device_id r block, ISO 8859/1 string d1 mfr_specific_01 r (UNKNOWN call syntax) d2 mfr_specific_02 r (UNKNOWN call syntax) d4 mfr_specific_04 r (UNKNOWN call syntax) d5 mfr_specific_05 r (UNKNOWN call syntax) d7 mfr_specific_07 rw (UNKNOWN call syntax) d8 mfr_specific_08 r (UNKNOWN call syntax) d9 mfr_specific_09 r (UNKNOWN call syntax) da mfr_specific_10 r (UNKNOWN call syntax) db mfr_specific_11 r (UNKNOWN call syntax) de mfr_specific_14 r (UNKNOWN call syntax) e0 mfr_specific_16 r (UNKNOWN call syntax) e1 mfr_specific_17 rw (UNKNOWN call syntax) e2 mfr_specific_18 rw (UNKNOWN call syntax) e3 mfr_specific_19 rw (UNKNOWN call syntax) e5 mfr_specific_21 w (UNKNOWN call syntax) f0 mfr_specific_32 rw (UNKNOWN call syntax) ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2025-04-19 17:53 ` Matt Corallo @ 2025-04-19 19:05 ` Guenter Roeck 2025-04-19 19:29 ` Matt Corallo 0 siblings, 1 reply; 29+ messages in thread From: Guenter Roeck @ 2025-04-19 19:05 UTC (permalink / raw) To: Matt Corallo; +Cc: linux-hwmon On 4/19/25 10:53, Matt Corallo wrote: > > > On 4/18/25 6:30 PM, Guenter Roeck wrote: >> On 4/18/25 14:03, Matt Corallo wrote: >>> >>> >>> On 4/17/25 9:21 PM, Guenter Roeck wrote: >>>> On 4/17/25 11:14, Matt Corallo wrote: >>>>> I do not, sadly (though FSP support has been rumored to help out at least marginally, though they haven't been useful for me). Interestingly the (I guess ancient now) pmbus_peek.c script has no issues reading from it (added a quick print on the -E2BIG line and it didn't get hit). pmbuss_peek.c says the following: >>>>> >>>>> root@rackchill-refresh:~# ./a.out -b /dev/i2c-3 -s 0x59 >>>>> PMBus slave on /dev/i2c-3, address 0x59 >>>>> >>>> >>>> pmbus_peek supports reading up to 255 bytes into the receive buffer. >>> >>> Hmm, I'm using this version, which on L622 checks for a length > 32 (and doesn't get hit in the -s command) - https://github.com/jktjkt/pmbus_peek/blob/master/pmbus_peek.c#L622 >>> >> >> Sorry, I am only guessing here. The code in pmbus_peek.c is a bit odd, though: >> It does validate the first byte, but then it jumps to try_i2c: and tries again, >> this time reading up to 255 bytes. I am not really sure if that ends up reporting >> an error or not. The comment "NOTE: this probably won't be visible" doesn't >> really improve my confidence that it will report the problem. > > Sure, that's why I `printf("BAD LEN\n");`'d there but didn't see it get hit. > > >> If you are up to compiling code, it would be great if you could add some debug logs >> into pmbus_peek.c and/or into the hid-cp2112.c driver to see what is actually >> returned from the chip. > > Sure, of course. Obviously easier to do in pmbus_peek if you have any suggested logs you want. The device caps output is (and still never hit my BAD LEN printf): > > ./a.out -b /dev/i2c-3 -l -v 0x58 > PMBus slave on /dev/i2c-3, address 0x58 > > Inventory Data: > Manufacturer: FSP-GROUP > Model: FSP520-20RAB > Revision: (null) > Built on: > Serial: > IC Device: PIC24FJ32GA004 > > PMBus revisions (0x22): part I, ver 1.1; part II, ver 1.2 > Capabilities (0x90): PEC, SMBALERT#, 100 KHz > > Supported Commands: > 00 page rw u8 (bitmask) > 01 operation rw u8 (bitmask) > 02 on_off_config rw u8 (bitmask) > 03 clear_fault w nodata > 05 page_plus_read w block > 19 capability r u8 (bitmask) > 1a query rw process_call pmbus_peek uses the query command to determine if a command is supported or not. It doesn't try to read other commands. > 1b smbalert_mask rw block > 20 vout_mode r u8 (bitmask) > 30 coefficients r process_call > 3b fan_command_1 rw s16 (LINEAR) > 79 status_word r u16 (bitmask) > 7a status_vout r u8 (bitmask) > 7b status_iout r u8 (bitmask) > 7c status_input r u8 (bitmask) > 7d status_temperature r u8 (bitmask) > 81 status_fans_1_2 rw u8 (bitmask) > 86 read_ein r block(6), Energy counter (DIRECT) > 87 read_eout r block(6), Energy counter (DIRECT) > 88 read_vin r s16 (LINEAR), Volts > 89 read_iin r s16 (LINEAR), Amperes > 8b read_vout r x16 (VOUT_MODE), Volts > 8c read_iout r s16 (LINEAR), Amperes > 8d read_temperature_1 r s16 (LINEAR), degrees Celsius > 8e read_temperature_2 r s16 (LINEAR), degrees Celsius > 90 read_fan_speed_1 r s16 (LINEAR) > 95 read_frequency r s16 (LINEAR) > 96 read_pout r s16 (LINEAR), Watts > 97 read_pin r s16 (LINEAR), Watts > 98 pmbus_revision r u8 (bitmask) > 99 mfr_id r block, ISO 8859/1 string > 9a mfr_model r block, ISO 8859/1 string > 9d mfr_date r block, ISO 8859/1 string > 9e mfr_serial r block, ISO 8859/1 string That also means it does not try to read mfr_revision or mfr_location since that is not supported. The PMBus driver in the kernel does try to read those (it doesn't use the query command to determine if a command is supported or not). My suspicion is that the power supply returns something (but not valid SMBus block data) when reading those commands. If modifying the kernel to find out is not easy, another option might be to enable smbus tracing. Would it be possible to do that ? Another option might be to modify the pmbus_peek command to read those registers, or maybe even better to use i2cget to read the data directly. Somewhat unrelated to this, it might be time to revisit using the QUERY command to determine what is supported. If only I had an endless amount of time ... oh well. When I wrote the kernel driver, none of the chips I had available at the time supported that command :-(. Thanks, Guenter ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2025-04-19 19:05 ` Guenter Roeck @ 2025-04-19 19:29 ` Matt Corallo 2025-04-19 22:38 ` Guenter Roeck 0 siblings, 1 reply; 29+ messages in thread From: Matt Corallo @ 2025-04-19 19:29 UTC (permalink / raw) To: Guenter Roeck; +Cc: linux-hwmon On 4/19/25 3:05 PM, Guenter Roeck wrote: > On 4/19/25 10:53, Matt Corallo wrote: >> >> Sure, of course. Obviously easier to do in pmbus_peek if you have any suggested logs you want. The >> device caps output is (and still never hit my BAD LEN printf): >> >> ./a.out -b /dev/i2c-3 -l -v 0x58 >> PMBus slave on /dev/i2c-3, address 0x58 >> >> Inventory Data: >> Manufacturer: FSP-GROUP >> Model: FSP520-20RAB >> Revision: (null) >> Built on: >> Serial: >> IC Device: PIC24FJ32GA004 >> >> PMBus revisions (0x22): part I, ver 1.1; part II, ver 1.2 >> Capabilities (0x90): PEC, SMBALERT#, 100 KHz >> >> Supported Commands: >> 00 page rw u8 (bitmask) >> 01 operation rw u8 (bitmask) >> 02 on_off_config rw u8 (bitmask) >> 03 clear_fault w nodata >> 05 page_plus_read w block >> 19 capability r u8 (bitmask) >> 1a query rw process_call > > pmbus_peek uses the query command to determine if a command > is supported or not. It doesn't try to read other commands. > >> 1b smbalert_mask rw block >> 20 vout_mode r u8 (bitmask) >> 30 coefficients r process_call >> 3b fan_command_1 rw s16 (LINEAR) >> 79 status_word r u16 (bitmask) >> 7a status_vout r u8 (bitmask) >> 7b status_iout r u8 (bitmask) >> 7c status_input r u8 (bitmask) >> 7d status_temperature r u8 (bitmask) >> 81 status_fans_1_2 rw u8 (bitmask) >> 86 read_ein r block(6), Energy counter (DIRECT) >> 87 read_eout r block(6), Energy counter (DIRECT) >> 88 read_vin r s16 (LINEAR), Volts >> 89 read_iin r s16 (LINEAR), Amperes >> 8b read_vout r x16 (VOUT_MODE), Volts >> 8c read_iout r s16 (LINEAR), Amperes >> 8d read_temperature_1 r s16 (LINEAR), degrees Celsius >> 8e read_temperature_2 r s16 (LINEAR), degrees Celsius >> 90 read_fan_speed_1 r s16 (LINEAR) >> 95 read_frequency r s16 (LINEAR) >> 96 read_pout r s16 (LINEAR), Watts >> 97 read_pin r s16 (LINEAR), Watts >> 98 pmbus_revision r u8 (bitmask) >> 99 mfr_id r block, ISO 8859/1 string >> 9a mfr_model r block, ISO 8859/1 string >> 9d mfr_date r block, ISO 8859/1 string >> 9e mfr_serial r block, ISO 8859/1 string > > That also means it does not try to read mfr_revision or mfr_location > since that is not supported. The PMBus driver in the kernel does > try to read those (it doesn't use the query command to determine > if a command is supported or not). > > My suspicion is that the power supply returns something (but not > valid SMBus block data) when reading those commands. If modifying > the kernel to find out is not easy, another option might be to > enable smbus tracing. Would it be possible to do that ? > Another option might be to modify the pmbus_peek command to read > those registers, or maybe even better to use i2cget to read the > data directly. Hmm, doesn't seem to trigger it at least with pmbus_peek.c, the following diff still doesn't hit the too big prints: diff -U5 orig.c pmbus_peek.c --- orig.c 2025-04-19 19:28:44.314887951 +0000 +++ pmbus_peek.c 2025-04-19 19:26:42.721194496 +0000 @@ -619,16 +619,19 @@ } if (data.block[0] <= read_len) { if (data.block[0] > 32) { /* NOTE: this probably won't be visible */ +fprintf(stderr, "\nERR too big 32!\n"); retval = -EFBIG; goto try_i2c; } retval = read_len = data.block[0]; - } else + } else { +fprintf(stderr, "\nERR too big!\n"); retval = -E2BIG; +} memcpy(read_buf, &data.block[1], read_len); return retval; try_i2c: @@ -1016,11 +1019,11 @@ * * NOTE: this assumes we can check prefixes for PMB_MFR_EXT(x) * and PMB_EXT(x) operations, even though we can't query those * operations directly... */ - if (is_pmb_extended(cmd)) + //if (is_pmb_extended(cmd)) return -1; if (pmdev->op[PMB_QUERY] == &unsupported || pmdev->no_query) return -1; @@ -1044,12 +1047,12 @@ int status; /* For non-queryable devices we'll still try to read inventory * data strings. That should be harmless but informative. */ - if (checksupport(pmdev, cmd) == 0) - return NULL; + //if (checksupport(pmdev, cmd) == 0) + // return NULL; memset(buf, 0, sizeof buf); status = pmbus_read_block(pmdev, cmd, sizeof buf - 1, buf); return (status > 0) ? strdup((void *)buf) : NULL; } > Somewhat unrelated to this, it might be time to revisit using the > QUERY command to determine what is supported. If only I had an endless > amount of time ... oh well. When I wrote the kernel driver, none of > the chips I had available at the time supported that command :-(. > > Thanks, > Guenter > > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2025-04-19 19:29 ` Matt Corallo @ 2025-04-19 22:38 ` Guenter Roeck 2025-04-19 22:49 ` Guenter Roeck 0 siblings, 1 reply; 29+ messages in thread From: Guenter Roeck @ 2025-04-19 22:38 UTC (permalink / raw) To: Matt Corallo; +Cc: linux-hwmon On 4/19/25 12:29, Matt Corallo wrote: > Hmm, doesn't seem to trigger it at least with pmbus_peek.c, the following diff still doesn't hit the too big prints: > Only idea I have at this point is to either enable smbus tracing in the kernel or (better) to add test code into i2c_smbus_read_block_data() to see what exactly triggers the problem. Guenter ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2025-04-19 22:38 ` Guenter Roeck @ 2025-04-19 22:49 ` Guenter Roeck 2025-04-20 2:29 ` Matt Corallo 0 siblings, 1 reply; 29+ messages in thread From: Guenter Roeck @ 2025-04-19 22:49 UTC (permalink / raw) To: Matt Corallo; +Cc: linux-hwmon On 4/19/25 15:38, Guenter Roeck wrote: > On 4/19/25 12:29, Matt Corallo wrote: > >> Hmm, doesn't seem to trigger it at least with pmbus_peek.c, the following diff still doesn't hit the too big prints: >> > > Only idea I have at this point is to either enable smbus tracing in the kernel > or (better) to add test code into i2c_smbus_read_block_data() to see what > exactly triggers the problem. > You could try this: diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c index e73afbefe222..b2856f88431d 100644 --- a/drivers/i2c/i2c-core-smbus.c +++ b/drivers/i2c/i2c-core-smbus.c @@ -233,6 +233,9 @@ s32 i2c_smbus_read_block_data(const struct i2c_client *client, u8 command, if (status) return status; + if (data.block[0] > I2C_SMBUS_BLOCK_MAX) + return -E2BIG; + memcpy(values, &data.block[1], data.block[0]); return data.block[0]; and maybe temporarily dump the entire buffer if that is seen. diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c index e73afbefe222..3dcb8b6b2427 100644 --- a/drivers/i2c/i2c-core-smbus.c +++ b/drivers/i2c/i2c-core-smbus.c @@ -233,6 +233,12 @@ s32 i2c_smbus_read_block_data(const struct i2c_client *client, u8 command, if (status) return status; + if (data.block[0] > I2C_SMBUS_BLOCK_MAX) { + print_hex_dump_bytes("Bad SMBus block data: ", DUMP_PREFIX_NONE, + data.block, I2C_SMBUS_BLOCK_MAX); + return -E2BIG; + } + memcpy(values, &data.block[1], data.block[0]); return data.block[0]; } If that doesn't trigger I am going to be really puzzled. Guenter ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2025-04-19 22:49 ` Guenter Roeck @ 2025-04-20 2:29 ` Matt Corallo 2025-04-20 3:03 ` Guenter Roeck 0 siblings, 1 reply; 29+ messages in thread From: Matt Corallo @ 2025-04-20 2:29 UTC (permalink / raw) To: Guenter Roeck; +Cc: linux-hwmon On 4/19/25 6:49 PM, Guenter Roeck wrote: > On 4/19/25 15:38, Guenter Roeck wrote: >> On 4/19/25 12:29, Matt Corallo wrote: >> >>> Hmm, doesn't seem to trigger it at least with pmbus_peek.c, the following diff still doesn't hit >>> the too big prints: >>> >> >> Only idea I have at this point is to either enable smbus tracing in the kernel >> or (better) to add test code into i2c_smbus_read_block_data() to see what >> exactly triggers the problem. >> > > You could try this: > > diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c > index e73afbefe222..b2856f88431d 100644 > --- a/drivers/i2c/i2c-core-smbus.c > +++ b/drivers/i2c/i2c-core-smbus.c > @@ -233,6 +233,9 @@ s32 i2c_smbus_read_block_data(const struct i2c_client *client, u8 command, > if (status) > return status; > > + if (data.block[0] > I2C_SMBUS_BLOCK_MAX) > + return -E2BIG; > + > memcpy(values, &data.block[1], data.block[0]); > return data.block[0]; > > and maybe temporarily dump the entire buffer if that is seen. > > diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c > index e73afbefe222..3dcb8b6b2427 100644 > --- a/drivers/i2c/i2c-core-smbus.c > +++ b/drivers/i2c/i2c-core-smbus.c > @@ -233,6 +233,12 @@ s32 i2c_smbus_read_block_data(const struct i2c_client *client, u8 command, > if (status) > return status; > > + if (data.block[0] > I2C_SMBUS_BLOCK_MAX) { > + print_hex_dump_bytes("Bad SMBus block data: ", DUMP_PREFIX_NONE, > + data.block, I2C_SMBUS_BLOCK_MAX); > + return -E2BIG; > + } > + > memcpy(values, &data.block[1], data.block[0]); > return data.block[0]; > } > > If that doesn't trigger I am going to be really puzzled. With this patch the device works fine. With debug enabled I see: [ 273.730583] i2c-core: driver [pmbus] registered [ 276.585423] pmbus 1-0058: probe [ 278.925878] Bad SMBus block data: ff ff b8 ff ff ff ff ff ff ff ff ff ff ff ff ff ................ [ 278.925881] Bad SMBus block data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ [ 278.937859] Bad SMBus block data: ff ff da ff ff ff ff ff ff ff ff ff ff ff ff ff ................ [ 278.937861] Bad SMBus block data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ [ 280.353885] i2c i2c-1: client [pmbus] registered with bus id 1-0058 [ 280.353888] i2c i2c-1: new_device: Instantiated device pmbus at 0x58 matt@psu-kern:~$ sensors pmbus-i2c-1-58 Adapter: CP2112 SMBus Bridge on hidraw1 vin: 123.00 V (crit min = -0.50 V, min = -0.50 V) ALARM (MIN) (max = -0.50 V, crit max = -0.50 V) vcap: -500.00 mV vout1: 12.05 V (crit min = +128.00 V, min = +128.00 V) (max = +128.00 V, crit max = +128.00 V) fan1: 5304 RPM fan2: -1 RPM fan3: FAULT ALARM fan4: FAULT ALARM temp1: +27.0°C (low = -0.5°C, high = -0.5°C) (crit low = -0.5°C, crit = -0.5°C) temp2: +32.0°C (low = -0.5°C, high = -0.5°C) (crit low = -0.5°C, crit = -0.5°C) temp3: -0.5°C (low = -0.5°C, high = -0.5°C) (crit low = -0.5°C, crit = -0.5°C) pin: 102.00 W (max = 1.60 kW) pout1: 88.00 W (max = 0.00 W, crit = -500.00 mW) (cap = -500.00 mW) iin: 988.00 mA (max = -0.01 A, crit max = -0.50 A) iout1: 7.39 A (crit min = -0.50 A, max = -0.50 A) (crit max = -0.50 A) ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2025-04-20 2:29 ` Matt Corallo @ 2025-04-20 3:03 ` Guenter Roeck 2025-04-25 8:16 ` Wolfram Sang 0 siblings, 1 reply; 29+ messages in thread From: Guenter Roeck @ 2025-04-20 3:03 UTC (permalink / raw) To: Matt Corallo; +Cc: linux-hwmon, Wolfram Sang, Linux I2C On 4/19/25 19:29, Matt Corallo wrote: > > > On 4/19/25 6:49 PM, Guenter Roeck wrote: >> On 4/19/25 15:38, Guenter Roeck wrote: >>> On 4/19/25 12:29, Matt Corallo wrote: >>> >>>> Hmm, doesn't seem to trigger it at least with pmbus_peek.c, the following diff still doesn't hit the too big prints: >>>> >>> >>> Only idea I have at this point is to either enable smbus tracing in the kernel >>> or (better) to add test code into i2c_smbus_read_block_data() to see what >>> exactly triggers the problem. >>> >> >> You could try this: >> >> diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c >> index e73afbefe222..b2856f88431d 100644 >> --- a/drivers/i2c/i2c-core-smbus.c >> +++ b/drivers/i2c/i2c-core-smbus.c >> @@ -233,6 +233,9 @@ s32 i2c_smbus_read_block_data(const struct i2c_client *client, u8 command, >> if (status) >> return status; >> >> + if (data.block[0] > I2C_SMBUS_BLOCK_MAX) >> + return -E2BIG; >> + >> memcpy(values, &data.block[1], data.block[0]); >> return data.block[0]; >> >> and maybe temporarily dump the entire buffer if that is seen. >> >> diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c >> index e73afbefe222..3dcb8b6b2427 100644 >> --- a/drivers/i2c/i2c-core-smbus.c >> +++ b/drivers/i2c/i2c-core-smbus.c >> @@ -233,6 +233,12 @@ s32 i2c_smbus_read_block_data(const struct i2c_client *client, u8 command, >> if (status) >> return status; >> >> + if (data.block[0] > I2C_SMBUS_BLOCK_MAX) { >> + print_hex_dump_bytes("Bad SMBus block data: ", DUMP_PREFIX_NONE, >> + data.block, I2C_SMBUS_BLOCK_MAX); >> + return -E2BIG; >> + } >> + >> memcpy(values, &data.block[1], data.block[0]); >> return data.block[0]; >> } >> >> If that doesn't trigger I am going to be really puzzled. > > With this patch the device works fine. With debug enabled I see: > > [ 273.730583] i2c-core: driver [pmbus] registered > [ 276.585423] pmbus 1-0058: probe > [ 278.925878] Bad SMBus block data: ff ff b8 ff ff ff ff ff ff ff ff ff ff ff ff ff ................ > [ 278.925881] Bad SMBus block data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ > [ 278.937859] Bad SMBus block data: ff ff da ff ff ff ff ff ff ff ff ff ff ff ff ff ................ > [ 278.937861] Bad SMBus block data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ > [ 280.353885] i2c i2c-1: client [pmbus] registered with bus id 1-0058 > [ 280.353888] i2c i2c-1: new_device: Instantiated device pmbus at 0x58 Great, thanks for confirming. So the two problems are: - The cp2112 driver does not validate the first data byte (the length field) when receiving SMBus block data from connected devices. - The I2C (SMBus) core does not validate the first data byte before copying the data buffer. Copying the I2C subsystem mailing list and the I2C maintainer. Wolfram, what do you suggest ? Fixing the cp2112 driver is obviously necessary, but I do wonder if a check such as the one above would be appropriate as well, possibly even combined with a WARN_ONCE(). Thanks, Guenter > matt@psu-kern:~$ sensors > pmbus-i2c-1-58 > Adapter: CP2112 SMBus Bridge on hidraw1 > vin: 123.00 V (crit min = -0.50 V, min = -0.50 V) ALARM (MIN) > (max = -0.50 V, crit max = -0.50 V) > vcap: -500.00 mV > vout1: 12.05 V (crit min = +128.00 V, min = +128.00 V) > (max = +128.00 V, crit max = +128.00 V) > fan1: 5304 RPM > fan2: -1 RPM > fan3: FAULT ALARM > fan4: FAULT ALARM > temp1: +27.0°C (low = -0.5°C, high = -0.5°C) > (crit low = -0.5°C, crit = -0.5°C) > temp2: +32.0°C (low = -0.5°C, high = -0.5°C) > (crit low = -0.5°C, crit = -0.5°C) > temp3: -0.5°C (low = -0.5°C, high = -0.5°C) > (crit low = -0.5°C, crit = -0.5°C) > pin: 102.00 W (max = 1.60 kW) > pout1: 88.00 W (max = 0.00 W, crit = -500.00 mW) > (cap = -500.00 mW) > iin: 988.00 mA (max = -0.01 A, crit max = -0.50 A) > iout1: 7.39 A (crit min = -0.50 A, max = -0.50 A) > (crit max = -0.50 A) ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2025-04-20 3:03 ` Guenter Roeck @ 2025-04-25 8:16 ` Wolfram Sang 2025-05-05 20:41 ` Matt Corallo 0 siblings, 1 reply; 29+ messages in thread From: Wolfram Sang @ 2025-04-25 8:16 UTC (permalink / raw) To: Guenter Roeck; +Cc: Matt Corallo, linux-hwmon, Linux I2C [-- Attachment #1: Type: text/plain, Size: 413 bytes --] > Wolfram, what do you suggest ? Fixing the cp2112 driver is obviously necessary, but > I do wonder if a check such as the one above would be appropriate as well, possibly > even combined with a WARN_ONCE(). How annoying, there was still an unchecked case left? Sorry. Yes, the core can have a check for a short-term solution. The long-term solution is to support SMBUS3.x which allows for 255 byte transfers. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2025-04-25 8:16 ` Wolfram Sang @ 2025-05-05 20:41 ` Matt Corallo 2025-05-05 20:50 ` Guenter Roeck 0 siblings, 1 reply; 29+ messages in thread From: Matt Corallo @ 2025-05-05 20:41 UTC (permalink / raw) To: Wolfram Sang, Guenter Roeck, linux-hwmon, Linux I2C On 4/25/25 4:16 AM, Wolfram Sang wrote: > >> Wolfram, what do you suggest ? Fixing the cp2112 driver is obviously necessary, but >> I do wonder if a check such as the one above would be appropriate as well, possibly >> even combined with a WARN_ONCE(). > > How annoying, there was still an unchecked case left? Sorry. Yes, the > core can have a check for a short-term solution. The long-term solution > is to support SMBUS3.x which allows for 255 byte transfers. Thanks! Any update here? I guess we already have a patch so no use in me trying to write one. Would be nice to get this in a pull so it can head through backports. Thanks, Matt ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2025-05-05 20:41 ` Matt Corallo @ 2025-05-05 20:50 ` Guenter Roeck 2025-05-05 20:57 ` Matt Corallo 0 siblings, 1 reply; 29+ messages in thread From: Guenter Roeck @ 2025-05-05 20:50 UTC (permalink / raw) To: Matt Corallo, Wolfram Sang, linux-hwmon, Linux I2C On 5/5/25 13:41, Matt Corallo wrote: > > > On 4/25/25 4:16 AM, Wolfram Sang wrote: >> >>> Wolfram, what do you suggest ? Fixing the cp2112 driver is obviously necessary, but >>> I do wonder if a check such as the one above would be appropriate as well, possibly >>> even combined with a WARN_ONCE(). >> >> How annoying, there was still an unchecked case left? Sorry. Yes, the >> core can have a check for a short-term solution. The long-term solution >> is to support SMBUS3.x which allows for 255 byte transfers. > > Thanks! > > Any update here? I guess we already have a patch so no use in me trying to write one. Would be nice to get this in a pull so it can head through backports. > Not from my side, sorry. I am deeply buried in work and don't have time for anything that isn't super-urgent :-( Guenter ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2025-05-05 20:50 ` Guenter Roeck @ 2025-05-05 20:57 ` Matt Corallo 2025-05-06 1:39 ` Guenter Roeck 0 siblings, 1 reply; 29+ messages in thread From: Matt Corallo @ 2025-05-05 20:57 UTC (permalink / raw) To: Guenter Roeck, Wolfram Sang, linux-hwmon, Linux I2C On 5/5/25 4:50 PM, Guenter Roeck wrote: > On 5/5/25 13:41, Matt Corallo wrote: >> >> >> On 4/25/25 4:16 AM, Wolfram Sang wrote: >>> >>>> Wolfram, what do you suggest ? Fixing the cp2112 driver is obviously necessary, but >>>> I do wonder if a check such as the one above would be appropriate as well, possibly >>>> even combined with a WARN_ONCE(). >>> >>> How annoying, there was still an unchecked case left? Sorry. Yes, the >>> core can have a check for a short-term solution. The long-term solution >>> is to support SMBUS3.x which allows for 255 byte transfers. >> >> Thanks! >> >> Any update here? I guess we already have a patch so no use in me trying to write one. Would be >> nice to get this in a pull so it can head through backports. >> > > Not from my side, sorry. I am deeply buried in work and don't have time for anything > that isn't super-urgent :-( Mmm, shame, its kinda annoying to leave a buffer overflow reachable from a malicious USB device sitting around (okay, with the default hardening configs it gets caught, but still). Can we just land the above patch from Wolfram to check the length before writing the buffer? Happy to clean it up as a formal patch submission if its easier for you. Thanks, Matt ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2025-05-05 20:57 ` Matt Corallo @ 2025-05-06 1:39 ` Guenter Roeck 2025-06-06 20:57 ` Matt Corallo 0 siblings, 1 reply; 29+ messages in thread From: Guenter Roeck @ 2025-05-06 1:39 UTC (permalink / raw) To: Matt Corallo, Wolfram Sang, linux-hwmon, Linux I2C On 5/5/25 13:57, Matt Corallo wrote: > > > On 5/5/25 4:50 PM, Guenter Roeck wrote: >> On 5/5/25 13:41, Matt Corallo wrote: >>> >>> >>> On 4/25/25 4:16 AM, Wolfram Sang wrote: >>>> >>>>> Wolfram, what do you suggest ? Fixing the cp2112 driver is obviously necessary, but >>>>> I do wonder if a check such as the one above would be appropriate as well, possibly >>>>> even combined with a WARN_ONCE(). >>>> >>>> How annoying, there was still an unchecked case left? Sorry. Yes, the >>>> core can have a check for a short-term solution. The long-term solution >>>> is to support SMBUS3.x which allows for 255 byte transfers. >>> >>> Thanks! >>> >>> Any update here? I guess we already have a patch so no use in me trying to write one. Would be nice to get this in a pull so it can head through backports. >>> >> >> Not from my side, sorry. I am deeply buried in work and don't have time for anything >> that isn't super-urgent :-( > > Mmm, shame, its kinda annoying to leave a buffer overflow reachable from a malicious USB device sitting around (okay, with the default hardening configs it gets caught, but still). Can we just land the above patch from Wolfram to check the length before writing the buffer? Happy to clean it up as a formal patch submission if its easier for you. > Please go ahead. Thanks, Guenter ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2025-05-06 1:39 ` Guenter Roeck @ 2025-06-06 20:57 ` Matt Corallo 2025-06-07 8:19 ` Greg KH 0 siblings, 1 reply; 29+ messages in thread From: Matt Corallo @ 2025-06-06 20:57 UTC (permalink / raw) To: Guenter Roeck, Wolfram Sang, linux-hwmon, Linux I2C, security Adding security@kernel.org cause probably they should make sure this gets fixed. On 5/5/25 9:39 PM, Guenter Roeck wrote: > On 5/5/25 13:57, Matt Corallo wrote: >> >> >> On 5/5/25 4:50 PM, Guenter Roeck wrote: >>> On 5/5/25 13:41, Matt Corallo wrote: >>>> >>>> >>>> On 4/25/25 4:16 AM, Wolfram Sang wrote: >>>>> >>>>>> Wolfram, what do you suggest ? Fixing the cp2112 driver is obviously necessary, but >>>>>> I do wonder if a check such as the one above would be appropriate as well, possibly >>>>>> even combined with a WARN_ONCE(). >>>>> >>>>> How annoying, there was still an unchecked case left? Sorry. Yes, the >>>>> core can have a check for a short-term solution. The long-term solution >>>>> is to support SMBUS3.x which allows for 255 byte transfers. >>>> >>>> Thanks! >>>> >>>> Any update here? I guess we already have a patch so no use in me trying to write one. Would be >>>> nice to get this in a pull so it can head through backports. >>>> >>> >>> Not from my side, sorry. I am deeply buried in work and don't have time for anything >>> that isn't super-urgent :-( >> >> Mmm, shame, its kinda annoying to leave a buffer overflow reachable from a malicious USB device >> sitting around (okay, with the default hardening configs it gets caught, but still). Can we just >> land the above patch from Wolfram to check the length before writing the buffer? Happy to clean it >> up as a formal patch submission if its easier for you. >> > > Please go ahead. > > Thanks, > Guenter > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2025-06-06 20:57 ` Matt Corallo @ 2025-06-07 8:19 ` Greg KH 2025-06-07 13:25 ` Matt Corallo 0 siblings, 1 reply; 29+ messages in thread From: Greg KH @ 2025-06-07 8:19 UTC (permalink / raw) To: Matt Corallo Cc: Guenter Roeck, Wolfram Sang, linux-hwmon, Linux I2C, security On Fri, Jun 06, 2025 at 04:57:37PM -0400, Matt Corallo wrote: > Adding security@kernel.org cause probably they should make sure this gets fixed. That's not how security@k.o works, sorry. As this is already public, no need for security@k.o to get involved at all, the normal development process happens here now. So, submit a patch and people will be glad to review it! thanks, greg k-h ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2025-06-07 8:19 ` Greg KH @ 2025-06-07 13:25 ` Matt Corallo 2025-06-08 7:14 ` Greg KH 0 siblings, 1 reply; 29+ messages in thread From: Matt Corallo @ 2025-06-07 13:25 UTC (permalink / raw) To: Greg KH; +Cc: Guenter Roeck, Wolfram Sang, linux-hwmon, Linux I2C, security On 6/7/25 4:19 AM, Greg KH wrote: > On Fri, Jun 06, 2025 at 04:57:37PM -0400, Matt Corallo wrote: >> Adding security@kernel.org cause probably they should make sure this gets fixed. > > That's not how security@k.o works, sorry. As this is already public, no > need for security@k.o to get involved at all, the normal development > process happens here now. > > So, submit a patch and people will be glad to review it! Thanks, figured I'd ask. Sadly there is a patch that folks seem to be okay with to fix a buffer overflow but its just sitting. Matt ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2025-06-07 13:25 ` Matt Corallo @ 2025-06-08 7:14 ` Greg KH 2025-06-09 13:57 ` Matt Corallo 0 siblings, 1 reply; 29+ messages in thread From: Greg KH @ 2025-06-08 7:14 UTC (permalink / raw) To: Matt Corallo Cc: Guenter Roeck, Wolfram Sang, linux-hwmon, Linux I2C, security On Sat, Jun 07, 2025 at 09:25:44AM -0400, Matt Corallo wrote: > > > On 6/7/25 4:19 AM, Greg KH wrote: > > On Fri, Jun 06, 2025 at 04:57:37PM -0400, Matt Corallo wrote: > > > Adding security@kernel.org cause probably they should make sure this gets fixed. > > > > That's not how security@k.o works, sorry. As this is already public, no > > need for security@k.o to get involved at all, the normal development > > process happens here now. > > > > So, submit a patch and people will be glad to review it! > > Thanks, figured I'd ask. Sadly there is a patch that folks seem to be okay > with to fix a buffer overflow but its just sitting. Have a pointer to that patch on lore for the maintainers involved to review? Note, we are in the middle of the merge window, so no new changes can be added to our trees until -rc1 is out. thanks, greg k-h ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2025-06-08 7:14 ` Greg KH @ 2025-06-09 13:57 ` Matt Corallo 2026-03-01 13:46 ` Matt Corallo 0 siblings, 1 reply; 29+ messages in thread From: Matt Corallo @ 2025-06-09 13:57 UTC (permalink / raw) To: Greg KH; +Cc: Guenter Roeck, Wolfram Sang, linux-hwmon, Linux I2C, security On 6/8/25 3:14 AM, Greg KH wrote: > On Sat, Jun 07, 2025 at 09:25:44AM -0400, Matt Corallo wrote: >> >> >> On 6/7/25 4:19 AM, Greg KH wrote: >>> On Fri, Jun 06, 2025 at 04:57:37PM -0400, Matt Corallo wrote: >>>> Adding security@kernel.org cause probably they should make sure this gets fixed. >>> >>> That's not how security@k.o works, sorry. As this is already public, no >>> need for security@k.o to get involved at all, the normal development >>> process happens here now. >>> >>> So, submit a patch and people will be glad to review it! >> >> Thanks, figured I'd ask. Sadly there is a patch that folks seem to be okay >> with to fix a buffer overflow but its just sitting. > > Have a pointer to that patch on lore for the maintainers involved to > review? Note, we are in the middle of the merge window, so no new > changes can be added to our trees until -rc1 is out. A proposed patch was posted by Guenter, and tested and confirmed that it fixes the issue by myself, at https://lore.kernel.org/linux-hwmon/284466fd-39e8-419e-8af5-41dbabb788af@roeck-us.net/ . Wolfram suggested this patch was acceptable at https://lore.kernel.org/linux-hwmon/aAtEydwUfVcE0XeA@shikoro/ but that's the last he chimed in on this issue. Matt ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2025-06-09 13:57 ` Matt Corallo @ 2026-03-01 13:46 ` Matt Corallo 2026-03-01 16:12 ` Kees Cook 0 siblings, 1 reply; 29+ messages in thread From: Matt Corallo @ 2026-03-01 13:46 UTC (permalink / raw) To: Greg KH; +Cc: Guenter Roeck, Wolfram Sang, linux-hwmon, Linux I2C, security On 6/9/25 9:57 AM, Matt Corallo wrote: > > > On 6/8/25 3:14 AM, Greg KH wrote: >> On Sat, Jun 07, 2025 at 09:25:44AM -0400, Matt Corallo wrote: >>> >>> >>> On 6/7/25 4:19 AM, Greg KH wrote: >>>> On Fri, Jun 06, 2025 at 04:57:37PM -0400, Matt Corallo wrote: >>>>> Adding security@kernel.org cause probably they should make sure this gets fixed. >>>> >>>> That's not how security@k.o works, sorry. As this is already public, no >>>> need for security@k.o to get involved at all, the normal development >>>> process happens here now. >>>> >>>> So, submit a patch and people will be glad to review it! >>> >>> Thanks, figured I'd ask. Sadly there is a patch that folks seem to be okay >>> with to fix a buffer overflow but its just sitting. >> >> Have a pointer to that patch on lore for the maintainers involved to >> review? Note, we are in the middle of the merge window, so no new >> changes can be added to our trees until -rc1 is out. > > A proposed patch was posted by Guenter, and tested and confirmed that it fixes the issue by myself, > at https://lore.kernel.org/linux-hwmon/284466fd-39e8-419e-8af5-41dbabb788af@roeck-us.net/ . Wolfram > suggested this patch was acceptable at https://lore.kernel.org/linux-hwmon/aAtEydwUfVcE0XeA@shikoro/ > but that's the last he chimed in on this issue. Any update on getting this patch applied Wolfram? Looks like the buffer overflow is still present on at least 6.18. Matt ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2026-03-01 13:46 ` Matt Corallo @ 2026-03-01 16:12 ` Kees Cook 2026-03-01 17:10 ` Matt Corallo 0 siblings, 1 reply; 29+ messages in thread From: Kees Cook @ 2026-03-01 16:12 UTC (permalink / raw) To: Matt Corallo, Greg KH Cc: Guenter Roeck, Wolfram Sang, linux-hwmon, Linux I2C, security On March 1, 2026 5:46:33 AM PST, Matt Corallo <yalbrymrb@mattcorallo.com> wrote: > > >On 6/9/25 9:57 AM, Matt Corallo wrote: >> >> >> On 6/8/25 3:14 AM, Greg KH wrote: >>> On Sat, Jun 07, 2025 at 09:25:44AM -0400, Matt Corallo wrote: >>>> >>>> >>>> On 6/7/25 4:19 AM, Greg KH wrote: >>>>> On Fri, Jun 06, 2025 at 04:57:37PM -0400, Matt Corallo wrote: >>>>>> Adding security@kernel.org cause probably they should make sure this gets fixed. >>>>> >>>>> That's not how security@k.o works, sorry. As this is already public, no >>>>> need for security@k.o to get involved at all, the normal development >>>>> process happens here now. >>>>> >>>>> So, submit a patch and people will be glad to review it! >>>> >>>> Thanks, figured I'd ask. Sadly there is a patch that folks seem to be okay >>>> with to fix a buffer overflow but its just sitting. >>> >>> Have a pointer to that patch on lore for the maintainers involved to >>> review? Note, we are in the middle of the merge window, so no new >>> changes can be added to our trees until -rc1 is out. >> >> A proposed patch was posted by Guenter, and tested and confirmed that it fixes the issue by myself, at https://lore.kernel.org/linux-hwmon/284466fd-39e8-419e-8af5-41dbabb788af@roeck-us.net/ . Wolfram suggested this patch was acceptable at https://lore.kernel.org/linux-hwmon/aAtEydwUfVcE0XeA@shikoro/ but that's the last he chimed in on this issue. > >Any update on getting this patch applied Wolfram? Looks like the buffer overflow is still present on at least 6.18. Looking at the code, I think probably the best place to check would be in i2c_smbus_read_block_data() when it does a I2C_SMBUS_BLOCK_DATA cmd, since the callers are all already checking the returned status. -- Kees Cook ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2026-03-01 16:12 ` Kees Cook @ 2026-03-01 17:10 ` Matt Corallo 2026-03-01 20:17 ` Guenter Roeck 2026-03-02 5:09 ` Kees Cook 0 siblings, 2 replies; 29+ messages in thread From: Matt Corallo @ 2026-03-01 17:10 UTC (permalink / raw) To: Kees Cook, Greg KH Cc: Guenter Roeck, Wolfram Sang, linux-hwmon, Linux I2C, security On 3/1/26 11:12 AM, Kees Cook wrote: > > > On March 1, 2026 5:46:33 AM PST, Matt Corallo <yalbrymrb@mattcorallo.com> wrote: >> >> >> On 6/9/25 9:57 AM, Matt Corallo wrote: >>> >>> >>> On 6/8/25 3:14 AM, Greg KH wrote: >>>> Have a pointer to that patch on lore for the maintainers involved to >>>> review? Note, we are in the middle of the merge window, so no new >>>> changes can be added to our trees until -rc1 is out. >>> >>> A proposed patch was posted by Guenter, and tested and confirmed that it fixes the issue by myself, at https://lore.kernel.org/linux-hwmon/284466fd-39e8-419e-8af5-41dbabb788af@roeck-us.net/ . Wolfram suggested this patch was acceptable at https://lore.kernel.org/linux-hwmon/aAtEydwUfVcE0XeA@shikoro/ but that's the last he chimed in on this issue. >> >> Any update on getting this patch applied Wolfram? Looks like the buffer overflow is still present on at least 6.18. > > Looking at the code, I think probably the best place to check would be in i2c_smbus_read_block_data() when it does a I2C_SMBUS_BLOCK_DATA cmd, since the callers are all already checking the returned status. I believe that's what the above patch does? Matt ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2026-03-01 17:10 ` Matt Corallo @ 2026-03-01 20:17 ` Guenter Roeck 2026-03-02 5:09 ` Kees Cook 1 sibling, 0 replies; 29+ messages in thread From: Guenter Roeck @ 2026-03-01 20:17 UTC (permalink / raw) To: Matt Corallo, Kees Cook, Greg KH Cc: Wolfram Sang, linux-hwmon, Linux I2C, security On 3/1/26 09:10, Matt Corallo wrote: > > > On 3/1/26 11:12 AM, Kees Cook wrote: >> >> >> On March 1, 2026 5:46:33 AM PST, Matt Corallo <yalbrymrb@mattcorallo.com> wrote: >>> >>> >>> On 6/9/25 9:57 AM, Matt Corallo wrote: >>>> >>>> >>>> On 6/8/25 3:14 AM, Greg KH wrote: >>>>> Have a pointer to that patch on lore for the maintainers involved to >>>>> review? Note, we are in the middle of the merge window, so no new >>>>> changes can be added to our trees until -rc1 is out. >>>> >>>> A proposed patch was posted by Guenter, and tested and confirmed that it fixes the issue by myself, at https://lore.kernel.org/linux-hwmon/284466fd-39e8-419e-8af5-41dbabb788af@roeck-us.net/ . Wolfram suggested this patch was acceptable at https://lore.kernel.org/linux-hwmon/aAtEydwUfVcE0XeA@shikoro/ but that's the last he chimed in on this issue. >>> >>> Any update on getting this patch applied Wolfram? Looks like the buffer overflow is still present on at least 6.18. >> >> Looking at the code, I think probably the best place to check would be in i2c_smbus_read_block_data() when it does a I2C_SMBUS_BLOCK_DATA cmd, since the callers are all already checking the returned status. > > I believe that's what the above patch does? > That wasn't a formal patch. I thought you had volunteered to submit one, but my memory may defeat me and I may be wrong. Guenter ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2026-03-01 17:10 ` Matt Corallo 2026-03-01 20:17 ` Guenter Roeck @ 2026-03-02 5:09 ` Kees Cook 2026-03-02 5:19 ` Guenter Roeck 1 sibling, 1 reply; 29+ messages in thread From: Kees Cook @ 2026-03-02 5:09 UTC (permalink / raw) To: Matt Corallo Cc: Greg KH, Guenter Roeck, Wolfram Sang, linux-hwmon, Linux I2C, security On Sun, Mar 01, 2026 at 12:10:08PM -0500, Matt Corallo wrote: > > > On 3/1/26 11:12 AM, Kees Cook wrote: > > > > > > On March 1, 2026 5:46:33 AM PST, Matt Corallo <yalbrymrb@mattcorallo.com> wrote: > > > > > > > > > On 6/9/25 9:57 AM, Matt Corallo wrote: > > > > > > > > > > > > On 6/8/25 3:14 AM, Greg KH wrote: > > > > > Have a pointer to that patch on lore for the maintainers involved to > > > > > review? Note, we are in the middle of the merge window, so no new > > > > > changes can be added to our trees until -rc1 is out. > > > > > > > > A proposed patch was posted by Guenter, and tested and confirmed that it fixes the issue by myself, at https://lore.kernel.org/linux-hwmon/284466fd-39e8-419e-8af5-41dbabb788af@roeck-us.net/ . Wolfram suggested this patch was acceptable at https://lore.kernel.org/linux-hwmon/aAtEydwUfVcE0XeA@shikoro/ but that's the last he chimed in on this issue. > > > > > > Any update on getting this patch applied Wolfram? Looks like the buffer overflow is still present on at least 6.18. > > > > Looking at the code, I think probably the best place to check would be in i2c_smbus_read_block_data() when it does a I2C_SMBUS_BLOCK_DATA cmd, since the callers are all already checking the returned status. > > I believe that's what the above patch does? Sorry, I mis-pasted. I meant within i2c_smbus_xfer (rather than i2c_smbus_read_block_data). Like: diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c index 71eb1ef56f0c..fbca606ba35a 100644 --- a/drivers/i2c/i2c-core-smbus.c +++ b/drivers/i2c/i2c-core-smbus.c @@ -547,6 +547,10 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr, command, protocol, data); i2c_unlock_bus(adapter, I2C_LOCK_SEGMENT); + if (WARN_ON_ONCE(res == 0 && command == I2C_SMBUS_BLOCK_DATA && + data->block[0] > I2C_SMBUS_BLOCK_MAX)) + res = -E2BIG; + return res; } EXPORT_SYMBOL(i2c_smbus_xfer); -- Kees Cook ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: PMBus memory overflow 2026-03-02 5:09 ` Kees Cook @ 2026-03-02 5:19 ` Guenter Roeck 0 siblings, 0 replies; 29+ messages in thread From: Guenter Roeck @ 2026-03-02 5:19 UTC (permalink / raw) To: Kees Cook, Matt Corallo Cc: Greg KH, Wolfram Sang, linux-hwmon, Linux I2C, security On 3/1/26 21:09, Kees Cook wrote: > On Sun, Mar 01, 2026 at 12:10:08PM -0500, Matt Corallo wrote: >> >> >> On 3/1/26 11:12 AM, Kees Cook wrote: >>> >>> >>> On March 1, 2026 5:46:33 AM PST, Matt Corallo <yalbrymrb@mattcorallo.com> wrote: >>>> >>>> >>>> On 6/9/25 9:57 AM, Matt Corallo wrote: >>>>> >>>>> >>>>> On 6/8/25 3:14 AM, Greg KH wrote: >>>>>> Have a pointer to that patch on lore for the maintainers involved to >>>>>> review? Note, we are in the middle of the merge window, so no new >>>>>> changes can be added to our trees until -rc1 is out. >>>>> >>>>> A proposed patch was posted by Guenter, and tested and confirmed that it fixes the issue by myself, at https://lore.kernel.org/linux-hwmon/284466fd-39e8-419e-8af5-41dbabb788af@roeck-us.net/ . Wolfram suggested this patch was acceptable at https://lore.kernel.org/linux-hwmon/aAtEydwUfVcE0XeA@shikoro/ but that's the last he chimed in on this issue. >>>> >>>> Any update on getting this patch applied Wolfram? Looks like the buffer overflow is still present on at least 6.18. >>> >>> Looking at the code, I think probably the best place to check would be in i2c_smbus_read_block_data() when it does a I2C_SMBUS_BLOCK_DATA cmd, since the callers are all already checking the returned status. >> >> I believe that's what the above patch does? > > Sorry, I mis-pasted. I meant within i2c_smbus_xfer (rather than > i2c_smbus_read_block_data). Like: > > > diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c > index 71eb1ef56f0c..fbca606ba35a 100644 > --- a/drivers/i2c/i2c-core-smbus.c > +++ b/drivers/i2c/i2c-core-smbus.c > @@ -547,6 +547,10 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr, > command, protocol, data); > i2c_unlock_bus(adapter, I2C_LOCK_SEGMENT); > > + if (WARN_ON_ONCE(res == 0 && command == I2C_SMBUS_BLOCK_DATA && > + data->block[0] > I2C_SMBUS_BLOCK_MAX)) > + res = -E2BIG; > + > return res; > } > EXPORT_SYMBOL(i2c_smbus_xfer); > Agreed, that makes more sense since it covers all callers, not just i2c_smbus_read_block_data(). Guenter ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2026-03-02 5:19 UTC | newest] Thread overview: 29+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-04-17 15:39 PMBus memory overflow Matt Corallo 2025-04-17 18:00 ` Guenter Roeck 2025-04-17 18:14 ` Matt Corallo 2025-04-18 1:21 ` Guenter Roeck 2025-04-18 21:03 ` Matt Corallo 2025-04-18 22:30 ` Guenter Roeck 2025-04-19 17:53 ` Matt Corallo 2025-04-19 19:05 ` Guenter Roeck 2025-04-19 19:29 ` Matt Corallo 2025-04-19 22:38 ` Guenter Roeck 2025-04-19 22:49 ` Guenter Roeck 2025-04-20 2:29 ` Matt Corallo 2025-04-20 3:03 ` Guenter Roeck 2025-04-25 8:16 ` Wolfram Sang 2025-05-05 20:41 ` Matt Corallo 2025-05-05 20:50 ` Guenter Roeck 2025-05-05 20:57 ` Matt Corallo 2025-05-06 1:39 ` Guenter Roeck 2025-06-06 20:57 ` Matt Corallo 2025-06-07 8:19 ` Greg KH 2025-06-07 13:25 ` Matt Corallo 2025-06-08 7:14 ` Greg KH 2025-06-09 13:57 ` Matt Corallo 2026-03-01 13:46 ` Matt Corallo 2026-03-01 16:12 ` Kees Cook 2026-03-01 17:10 ` Matt Corallo 2026-03-01 20:17 ` Guenter Roeck 2026-03-02 5:09 ` Kees Cook 2026-03-02 5:19 ` Guenter Roeck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox