[PATCH] i2c: amd-mp2: Synchronize IRQ handler on callback unregister

Fan Wu posted 1 patch 1 day, 5 hours ago
drivers/i2c/busses/i2c-amd-mp2-pci.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
[PATCH] i2c: amd-mp2: Synchronize IRQ handler on callback unregister
Posted by Fan Wu 1 day, 5 hours ago
amd_mp2_irq_isr() dereferences the per-bus i2c context stored in
privdata->busses[] and calls its cmd_completion() callback.
i2c_amd_remove() clears that pointer with amd_mp2_unregister_cb()
without any synchronization, so a handler that already loaded the
pointer can still write i2c_common->eventval and call
cmd_completion() after devres frees the struct amd_i2c_dev embedding
the context.

Fix this by calling synchronize_irq() in amd_mp2_unregister_cb()
after the pointer is cleared: handler instances that started earlier
finish before the caller frees the object, and handlers that run
later observe the NULL entry and skip the bus.

The interrupt handler keeps reading the slot without any lock, so the
clear and the handler's read use WRITE_ONCE()/READ_ONCE() to mark
the access as intentionally concurrent.

synchronize_irq() can sleep, but its callers hold only the sleepable
adapter bus lock, which the interrupt handler never takes.

This issue was found by an in-house static analysis tool.

Fixes: 529766e0a011 ("i2c: Add drivers for the AMD PCIe MP2 I2C controller")
Cc: stable@vger.kernel.org
Co-developed-by: Song Li <songl@zju.edu.cn>
Signed-off-by: Song Li <songl@zju.edu.cn>
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
 drivers/i2c/busses/i2c-amd-mp2-pci.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-amd-mp2-pci.c b/drivers/i2c/busses/i2c-amd-mp2-pci.c
index 60edbab..8c81247 100644
--- a/drivers/i2c/busses/i2c-amd-mp2-pci.c
+++ b/drivers/i2c/busses/i2c-amd-mp2-pci.c
@@ -209,7 +209,7 @@ static irqreturn_t amd_mp2_irq_isr(int irq, void *dev)
 	enum irqreturn ret = IRQ_NONE;
 
 	for (bus_id = 0; bus_id < 2; bus_id++) {
-		i2c_common = privdata->busses[bus_id];
+		i2c_common = READ_ONCE(privdata->busses[bus_id]);
 		if (!i2c_common)
 			continue;
 
@@ -268,7 +268,9 @@ int amd_mp2_unregister_cb(struct amd_i2c_common *i2c_common)
 {
 	struct amd_mp2_dev *privdata = i2c_common->mp2_dev;
 
-	privdata->busses[i2c_common->bus_id] = NULL;
+	WRITE_ONCE(privdata->busses[i2c_common->bus_id], NULL);
+
+	synchronize_irq(privdata->dev_irq);
 
 	return 0;
 }