* [PATCH] i2c: i801: Revert "i2c: i801: replace acpi_lock with I2C bus lock"
@ 2026-02-28 1:41 Charles Haithcock
2026-03-04 11:47 ` Wolfram Sang
0 siblings, 1 reply; 2+ messages in thread
From: Charles Haithcock @ 2026-02-28 1:41 UTC (permalink / raw)
To: jdelvare, andi.shyti; +Cc: Charles Haithcock, linux-i2c
This reverts commit f707d6b9e7c18f669adfdb443906d46cfbaaa0c1.
Under rare circumstances, multiple udev threads can collect i801 device
info on boot and walk i801_acpi_io_handler somewhat concurrently. The
first will note the area is reserved by acpi to prevent further touches.
This ultimately causes the area to be deregistered. The second will
enter i801_acpi_io_handler after the area is unregistered but before a
check can be made that the area is unregistered. i2c_lock_bus relies on
the now unregistered area containing lock_ops to lock the bus. The end
result is a kernel panic on boot with the following backtrace;
[ 14.971872] ioatdma 0000:09:00.2: enabling device (0100 -> 0102)
[ 14.971873] BUG: kernel NULL pointer dereference, address: 0000000000000000
[ 14.971880] #PF: supervisor read access in kernel mode
[ 14.971884] #PF: error_code(0x0000) - not-present page
[ 14.971887] PGD 0 P4D 0
[ 14.971894] Oops: 0000 [#1] PREEMPT SMP PTI
[ 14.971900] CPU: 5 PID: 956 Comm: systemd-udevd Not tainted 5.14.0-611.5.1.el9_7.x86_64 #1
[ 14.971905] Hardware name: XXXXXXXXXXXXXXXXXXXXXXX BIOS 1.20.10.SV91 01/30/2023
[ 14.971908] RIP: 0010:i801_acpi_io_handler+0x2d/0xb0 [i2c_i801]
[ 14.971929] Code: 00 00 49 8b 40 20 41 57 41 56 4d 8b b8 30 04 00 00 49 89 ce 41 55 41 89 d5 41 54 49 89 f4 be 02 00 00 00 55 4c 89 c5 53 89 fb <48> 8b 00 4c 89 c7 e8 18 61 54 e9 80 bd 80 04 00 00 00 75 09 4c 3b
[ 14.971933] RSP: 0018:ffffbaa841483838 EFLAGS: 00010282
[ 14.971938] RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffff9685e01ba568
[ 14.971941] RDX: 0000000000000008 RSI: 0000000000000002 RDI: 0000000000000000
[ 14.971944] RBP: ffff9685ca22f028 R08: ffff9685ca22f028 R09: ffff9685ca22f028
[ 14.971948] R10: 000000000000000b R11: 0000000000000580 R12: 0000000000000580
[ 14.971951] R13: 0000000000000008 R14: ffff9685e01ba568 R15: ffff9685c222f000
[ 14.971954] FS: 00007f8287c0ab40(0000) GS:ffff96a47f940000(0000) knlGS:0000000000000000
[ 14.971959] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 14.971963] CR2: 0000000000000000 CR3: 0000000168090001 CR4: 00000000003706f0
[ 14.971966] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 14.971968] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 14.971972] Call Trace:
[ 14.971977] <TASK>
[ 14.971981] ? show_trace_log_lvl+0x1c4/0x2df
[ 14.971994] ? show_trace_log_lvl+0x1c4/0x2df
[ 14.972003] ? acpi_ev_address_space_dispatch+0x16e/0x3c0
[ 14.972014] ? __die_body.cold+0x8/0xd
[ 14.972021] ? page_fault_oops+0x132/0x170
[ 14.972028] ? exc_page_fault+0x61/0x150
[ 14.972036] ? asm_exc_page_fault+0x22/0x30
[ 14.972045] ? i801_acpi_io_handler+0x2d/0xb0 [i2c_i801]
[ 14.972061] acpi_ev_address_space_dispatch+0x16e/0x3c0
[ 14.972069] ? __pfx_i801_acpi_io_handler+0x10/0x10 [i2c_i801]
[ 14.972085] acpi_ex_access_region+0x5b/0xd0
[ 14.972093] acpi_ex_field_datum_io+0x73/0x2e0
[ 14.972100] acpi_ex_read_data_from_field+0x8e/0x230
[ 14.972106] acpi_ex_resolve_node_to_value+0x23d/0x310
[ 14.972114] acpi_ds_evaluate_name_path+0xad/0x110
[ 14.972121] acpi_ds_exec_end_op+0x321/0x510
[ 14.972127] acpi_ps_parse_loop+0xf7/0x680
[ 14.972136] acpi_ps_parse_aml+0x17a/0x3d0
[ 14.972143] acpi_ps_execute_method+0x137/0x270
[ 14.972150] acpi_ns_evaluate+0x1f4/0x2e0
[ 14.972158] acpi_evaluate_object+0x134/0x2f0
[ 14.972164] acpi_evaluate_integer+0x50/0xe0
[ 14.972173] ? vsnprintf+0x24b/0x570
[ 14.972181] acpi_ac_get_state.part.0+0x23/0x70
[ 14.972189] get_ac_property+0x4e/0x60
[ 14.972195] power_supply_show_property+0x90/0x1f0
[ 14.972205] add_prop_uevent+0x29/0x90
[ 14.972213] power_supply_uevent+0x109/0x1d0
[ 14.972222] dev_uevent+0x10e/0x2f0
[ 14.972228] uevent_show+0x8e/0x100
[ 14.972236] dev_attr_show+0x19/0x40
[ 14.972246] sysfs_kf_seq_show+0x9b/0x100
[ 14.972253] seq_read_iter+0x120/0x4b0
[ 14.972262] ? selinux_file_permission+0x106/0x150
[ 14.972273] vfs_read+0x24f/0x3a0
[ 14.972284] ksys_read+0x5f/0xe0
[ 14.972291] do_syscall_64+0x5f/0xe0
...
The kernel panic is mitigated by setting limiting the count of udev
children to 1. Revert to using the acpi_lock to continue protecting
marking the area as owned by firmware without relying on a lock in
a potentially unmapped region of memory.
Signed-off-by: Charles Haithcock <chaithco@redhat.com>
---
drivers/i2c/busses/i2c-i801.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 9e1789725e..0ff3edccce 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -310,9 +310,10 @@ struct i801_priv {
/*
* If set to true the host controller registers are reserved for
- * ACPI AML use.
+ * ACPI AML use. Protected by acpi_lock.
*/
bool acpi_reserved;
+ struct mutex acpi_lock;
};
#define FEATURE_SMBUS_PEC BIT(0)
@@ -894,8 +895,11 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
int hwpec, ret;
struct i801_priv *priv = i2c_get_adapdata(adap);
- if (priv->acpi_reserved)
+ mutex_lock(&priv->acpi_lock);
+ if (priv->acpi_reserved) {
+ mutex_unlock(&priv->acpi_lock);
return -EBUSY;
+ }
pm_runtime_get_sync(&priv->pci_dev->dev);
@@ -935,6 +939,7 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
iowrite8(SMBHSTSTS_INUSE_STS | STATUS_FLAGS, SMBHSTSTS(priv));
pm_runtime_put_autosuspend(&priv->pci_dev->dev);
+ mutex_unlock(&priv->acpi_lock);
return ret;
}
@@ -1465,7 +1470,7 @@ i801_acpi_io_handler(u32 function, acpi_physical_address address, u32 bits,
* further access from the driver itself. This device is now owned
* by the system firmware.
*/
- i2c_lock_bus(&priv->adapter, I2C_LOCK_SEGMENT);
+ mutex_lock(&priv->acpi_lock);
if (!priv->acpi_reserved && i801_acpi_is_smbus_ioport(priv, address)) {
priv->acpi_reserved = true;
@@ -1485,7 +1490,7 @@ i801_acpi_io_handler(u32 function, acpi_physical_address address, u32 bits,
else
status = acpi_os_write_port(address, (u32)*value, bits);
- i2c_unlock_bus(&priv->adapter, I2C_LOCK_SEGMENT);
+ mutex_unlock(&priv->acpi_lock);
return status;
}
@@ -1545,6 +1550,7 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
priv->adapter.dev.parent = &dev->dev;
acpi_use_parent_companion(&priv->adapter.dev);
priv->adapter.retries = 3;
+ mutex_init(&priv->acpi_lock);
priv->pci_dev = dev;
priv->features = id->driver_data;
--
2.53.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] i2c: i801: Revert "i2c: i801: replace acpi_lock with I2C bus lock"
2026-02-28 1:41 [PATCH] i2c: i801: Revert "i2c: i801: replace acpi_lock with I2C bus lock" Charles Haithcock
@ 2026-03-04 11:47 ` Wolfram Sang
0 siblings, 0 replies; 2+ messages in thread
From: Wolfram Sang @ 2026-03-04 11:47 UTC (permalink / raw)
To: Charles Haithcock; +Cc: jdelvare, andi.shyti, linux-i2c
On Fri, Feb 27, 2026 at 06:41:15PM -0700, Charles Haithcock wrote:
> This reverts commit f707d6b9e7c18f669adfdb443906d46cfbaaa0c1.
>
> Under rare circumstances, multiple udev threads can collect i801 device
> info on boot and walk i801_acpi_io_handler somewhat concurrently. The
> first will note the area is reserved by acpi to prevent further touches.
> This ultimately causes the area to be deregistered. The second will
> enter i801_acpi_io_handler after the area is unregistered but before a
> check can be made that the area is unregistered. i2c_lock_bus relies on
> the now unregistered area containing lock_ops to lock the bus. The end
> result is a kernel panic on boot with the following backtrace;
>
> [ 14.971872] ioatdma 0000:09:00.2: enabling device (0100 -> 0102)
> [ 14.971873] BUG: kernel NULL pointer dereference, address: 0000000000000000
> [ 14.971880] #PF: supervisor read access in kernel mode
> [ 14.971884] #PF: error_code(0x0000) - not-present page
> [ 14.971887] PGD 0 P4D 0
> [ 14.971894] Oops: 0000 [#1] PREEMPT SMP PTI
> [ 14.971900] CPU: 5 PID: 956 Comm: systemd-udevd Not tainted 5.14.0-611.5.1.el9_7.x86_64 #1
> [ 14.971905] Hardware name: XXXXXXXXXXXXXXXXXXXXXXX BIOS 1.20.10.SV91 01/30/2023
> [ 14.971908] RIP: 0010:i801_acpi_io_handler+0x2d/0xb0 [i2c_i801]
> [ 14.971929] Code: 00 00 49 8b 40 20 41 57 41 56 4d 8b b8 30 04 00 00 49 89 ce 41 55 41 89 d5 41 54 49 89 f4 be 02 00 00 00 55 4c 89 c5 53 89 fb <48> 8b 00 4c 89 c7 e8 18 61 54 e9 80 bd 80 04 00 00 00 75 09 4c 3b
> [ 14.971933] RSP: 0018:ffffbaa841483838 EFLAGS: 00010282
> [ 14.971938] RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffff9685e01ba568
> [ 14.971941] RDX: 0000000000000008 RSI: 0000000000000002 RDI: 0000000000000000
> [ 14.971944] RBP: ffff9685ca22f028 R08: ffff9685ca22f028 R09: ffff9685ca22f028
> [ 14.971948] R10: 000000000000000b R11: 0000000000000580 R12: 0000000000000580
> [ 14.971951] R13: 0000000000000008 R14: ffff9685e01ba568 R15: ffff9685c222f000
> [ 14.971954] FS: 00007f8287c0ab40(0000) GS:ffff96a47f940000(0000) knlGS:0000000000000000
> [ 14.971959] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 14.971963] CR2: 0000000000000000 CR3: 0000000168090001 CR4: 00000000003706f0
> [ 14.971966] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 14.971968] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 14.971972] Call Trace:
> [ 14.971977] <TASK>
> [ 14.971981] ? show_trace_log_lvl+0x1c4/0x2df
> [ 14.971994] ? show_trace_log_lvl+0x1c4/0x2df
> [ 14.972003] ? acpi_ev_address_space_dispatch+0x16e/0x3c0
> [ 14.972014] ? __die_body.cold+0x8/0xd
> [ 14.972021] ? page_fault_oops+0x132/0x170
> [ 14.972028] ? exc_page_fault+0x61/0x150
> [ 14.972036] ? asm_exc_page_fault+0x22/0x30
> [ 14.972045] ? i801_acpi_io_handler+0x2d/0xb0 [i2c_i801]
> [ 14.972061] acpi_ev_address_space_dispatch+0x16e/0x3c0
> [ 14.972069] ? __pfx_i801_acpi_io_handler+0x10/0x10 [i2c_i801]
> [ 14.972085] acpi_ex_access_region+0x5b/0xd0
> [ 14.972093] acpi_ex_field_datum_io+0x73/0x2e0
> [ 14.972100] acpi_ex_read_data_from_field+0x8e/0x230
> [ 14.972106] acpi_ex_resolve_node_to_value+0x23d/0x310
> [ 14.972114] acpi_ds_evaluate_name_path+0xad/0x110
> [ 14.972121] acpi_ds_exec_end_op+0x321/0x510
> [ 14.972127] acpi_ps_parse_loop+0xf7/0x680
> [ 14.972136] acpi_ps_parse_aml+0x17a/0x3d0
> [ 14.972143] acpi_ps_execute_method+0x137/0x270
> [ 14.972150] acpi_ns_evaluate+0x1f4/0x2e0
> [ 14.972158] acpi_evaluate_object+0x134/0x2f0
> [ 14.972164] acpi_evaluate_integer+0x50/0xe0
> [ 14.972173] ? vsnprintf+0x24b/0x570
> [ 14.972181] acpi_ac_get_state.part.0+0x23/0x70
> [ 14.972189] get_ac_property+0x4e/0x60
> [ 14.972195] power_supply_show_property+0x90/0x1f0
> [ 14.972205] add_prop_uevent+0x29/0x90
> [ 14.972213] power_supply_uevent+0x109/0x1d0
> [ 14.972222] dev_uevent+0x10e/0x2f0
> [ 14.972228] uevent_show+0x8e/0x100
> [ 14.972236] dev_attr_show+0x19/0x40
> [ 14.972246] sysfs_kf_seq_show+0x9b/0x100
> [ 14.972253] seq_read_iter+0x120/0x4b0
> [ 14.972262] ? selinux_file_permission+0x106/0x150
> [ 14.972273] vfs_read+0x24f/0x3a0
> [ 14.972284] ksys_read+0x5f/0xe0
> [ 14.972291] do_syscall_64+0x5f/0xe0
> ...
>
> The kernel panic is mitigated by setting limiting the count of udev
> children to 1. Revert to using the acpi_lock to continue protecting
> marking the area as owned by firmware without relying on a lock in
> a potentially unmapped region of memory.
>
> Signed-off-by: Charles Haithcock <chaithco@redhat.com>
Boy, this was probably a hard one, kudos for fixing it and sorry about
the regression. I added a fixes tag and improved the original comment to
emphasize the importance of the lock.
Applied to for-current, thanks!
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-03-04 11:47 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-28 1:41 [PATCH] i2c: i801: Revert "i2c: i801: replace acpi_lock with I2C bus lock" Charles Haithcock
2026-03-04 11:47 ` Wolfram Sang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox