i3c: mipi-i3c-hci: Fix race in i3c_hci_addr_to_dev()

i3c_hci_addr_to_dev() walks bus->devs.i3c, which is protected by
bus.lock (rwsem).  However, it is invoked from the MIPI I3C HCI IRQ
handler, which cannot take bus.lock.  This allows concurrent device
addition/removal in the I3C core to modify the list while it is being
traversed, potentially leading to use-after-free or crashes.

Remove the dependency on the bus device list and introduce a dedicated
lookup table.  Add an ibi_devs[] array indexed by DAT entry, maintained
under hci->lock.  Update the array when IBIs are enabled or disabled,
so that it always reflects the set of devices allowed to generate IBIs.
Also update when IBIs are freed, to cover the corner case when an IBI is
freed without first being disabled (e.g. oldedev in
i3c_master_add_i3c_dev_locked()).

Move i3c_hci_addr_to_dev() into core.c, reimplement it using the new
array, and add a lockdep assertion to enforce that hci->lock is held
by callers.

Demote a message in PIO and DMA IBI handling, from an error to a debug
message, because there is a race window when the condition can arise
normally.

Fixes: 9ad9a52cce282 ("i3c/master: introduce the mipi-i3c-hci driver")
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Link: https://patch.msgid.link/20260612080107.11606-2-adrian.hunter@intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
index 53797841b63f9132ffc4bf7d5e4c5d4de9c0a153..1e1f05aff0927f5fbe0e426555f4177c0bdad4d9 100644 (file)
--- a/drivers/i3c/master/mipi-i3c-hci/core.c
+++ b/drivers/i3c/master/mipi-i3c-hci/core.c
@@ -22,6 +22,7 @@
 #include "ext_caps.h"
 #include "cmd.h"
 #include "dat.h"
+#include "ibi.h"
 
 /*
  * Host Controller Capabilities and Operation Registers
@@ -124,6 +125,7 @@ static void i3c_hci_set_master_dyn_addr(struct i3c_hci *hci)
 static int i3c_hci_bus_init(struct i3c_master_controller *m)
 {
        struct i3c_hci *hci = to_i3c_hci(m);
+       struct device *dev = hci->master.dev.parent;
        struct i3c_device_info info;
        int ret;
 
@@ -144,6 +146,10 @@ static int i3c_hci_bus_init(struct i3c_master_controller *m)
        if (ret)
                return ret;
 
+       hci->ibi_devs = devm_kcalloc(dev, hci->DAT_entries, sizeof(*hci->ibi_devs), GFP_KERNEL);
+       if (!hci->ibi_devs)
+               return -ENOMEM;
+
        ret = hci->io->init(hci);
        if (ret)
                return ret;
@@ -639,14 +645,40 @@ static int i3c_hci_request_ibi(struct i3c_dev_desc *dev,
        return hci->io->request_ibi(hci, dev, req);
 }
 
+static void __i3c_hci_disable_ibi(struct i3c_hci *hci, struct i3c_dev_desc *dev)
+{
+       struct i3c_hci_dev_data *dev_data = i3c_dev_get_master_data(dev);
+
+       mipi_i3c_hci_dat_v1.set_flags(hci, dev_data->dat_idx, DAT_0_SIR_REJECT, 0);
+       scoped_guard(spinlock_irqsave, &hci->lock)
+               hci->ibi_devs[dev_data->dat_idx] = NULL;
+}
+
 static void i3c_hci_free_ibi(struct i3c_dev_desc *dev)
 {
        struct i3c_master_controller *m = i3c_dev_get_master(dev);
        struct i3c_hci *hci = to_i3c_hci(m);
 
+       /* Must ensure the IBI has been disabled */
+       __i3c_hci_disable_ibi(hci, dev);
        hci->io->free_ibi(hci, dev);
 }
 
+struct i3c_dev_desc *i3c_hci_addr_to_dev(struct i3c_hci *hci, unsigned int addr)
+{
+       int dat_idx;
+
+       lockdep_assert_held(&hci->lock);
+
+       for (dat_idx = 0; dat_idx < hci->DAT_entries; dat_idx++) {
+               struct i3c_dev_desc *dev = hci->ibi_devs[dat_idx];
+
+               if (dev && dev->info.dyn_addr == addr)
+                       return dev;
+       }
+       return NULL;
+}
+
 static int i3c_hci_enable_ibi(struct i3c_dev_desc *dev)
 {
        struct i3c_master_controller *m = i3c_dev_get_master(dev);
@@ -654,6 +686,8 @@ static int i3c_hci_enable_ibi(struct i3c_dev_desc *dev)
        struct i3c_hci_dev_data *dev_data = i3c_dev_get_master_data(dev);
 
        mipi_i3c_hci_dat_v1.clear_flags(hci, dev_data->dat_idx, DAT_0_SIR_REJECT, 0);
+       scoped_guard(spinlock_irqsave, &hci->lock)
+               hci->ibi_devs[dev_data->dat_idx] = dev;
        return i3c_master_enec_locked(m, dev->info.dyn_addr, I3C_CCC_EVENT_SIR);
 }
 
@@ -661,9 +695,8 @@ static int i3c_hci_disable_ibi(struct i3c_dev_desc *dev)
 {
        struct i3c_master_controller *m = i3c_dev_get_master(dev);
        struct i3c_hci *hci = to_i3c_hci(m);
-       struct i3c_hci_dev_data *dev_data = i3c_dev_get_master_data(dev);
 
-       mipi_i3c_hci_dat_v1.set_flags(hci, dev_data->dat_idx, DAT_0_SIR_REJECT, 0);
+       __i3c_hci_disable_ibi(hci, dev);
        return i3c_master_disec_locked(m, dev->info.dyn_addr, I3C_CCC_EVENT_SIR);
 }
 

diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
index 87622d6f817e8e0d568a62af2ac1396c5e7f015a..0672ed1132f8f5806de930c892c082869f74510e 100644 (file)
--- a/drivers/i3c/master/mipi-i3c-hci/dma.c
+++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
@@ -967,8 +967,11 @@ static void hci_dma_process_ibi(struct i3c_hci *hci, struct hci_rh_data *rh)
 
        dev = i3c_hci_addr_to_dev(hci, ibi_addr);
        if (!dev) {
-               dev_err(&hci->master.dev,
-                       "IBI for unknown device %#x\n", ibi_addr);
+               /*
+                * Either an IBI received just before IBI's were disabled, or
+                * the controller is broken. Assume the former.
+                */
+               dev_dbg(&hci->master.dev, "IBI when not enabled at address %#x\n", ibi_addr);
                goto done;
        }
 

diff --git a/drivers/i3c/master/mipi-i3c-hci/hci.h b/drivers/i3c/master/mipi-i3c-hci/hci.h
index 41d31a53abd370cf5e1d635ecc9af0a0d0cecafe..b3d9803b19680aea243903937cff83c91a7d8681 100644 (file)
--- a/drivers/i3c/master/mipi-i3c-hci/hci.h
+++ b/drivers/i3c/master/mipi-i3c-hci/hci.h
@@ -65,6 +65,7 @@ struct i3c_hci {
        unsigned int DAT_entry_size;
        void *DAT_data;
        struct dat_words *DAT;
+       struct i3c_dev_desc **ibi_devs;
        unsigned int DCT_entries;
        unsigned int DCT_entry_size;
        u8 version_major;

diff --git a/drivers/i3c/master/mipi-i3c-hci/ibi.h b/drivers/i3c/master/mipi-i3c-hci/ibi.h
index e1f98e264da0ebe52e384f7a33324c10f77667f1..073ca67b7d04d15855cd0e8664607b98f3e71f2d 100644 (file)
--- a/drivers/i3c/master/mipi-i3c-hci/ibi.h
+++ b/drivers/i3c/master/mipi-i3c-hci/ibi.h
@@ -26,17 +26,6 @@
 #define IBI_DATA_LENGTH                        GENMASK(7, 0)
 
 /*  handy helpers */
-static inline struct i3c_dev_desc *
-i3c_hci_addr_to_dev(struct i3c_hci *hci, unsigned int addr)
-{
-       struct i3c_bus *bus = i3c_master_get_bus(&hci->master);
-       struct i3c_dev_desc *dev;
-
-       i3c_bus_for_each_i3cdev(bus, dev) {
-               if (dev->info.dyn_addr == addr)
-                       return dev;
-       }
-       return NULL;
-}
+struct i3c_dev_desc *i3c_hci_addr_to_dev(struct i3c_hci *hci, unsigned int addr);
 
 #endif

diff --git a/drivers/i3c/master/mipi-i3c-hci/pio.c b/drivers/i3c/master/mipi-i3c-hci/pio.c
index b5ae1cfaa9e03d8238ff09d14796b5320690e81b..ff2657ee220ba86908364a9c8ee5a1a208522c45 100644 (file)
--- a/drivers/i3c/master/mipi-i3c-hci/pio.c
+++ b/drivers/i3c/master/mipi-i3c-hci/pio.c
@@ -869,8 +869,11 @@ static bool hci_pio_prep_new_ibi(struct i3c_hci *hci, struct hci_pio_data *pio)
 
        dev = i3c_hci_addr_to_dev(hci, ibi->addr);
        if (!dev) {
-               dev_err(&hci->master.dev,
-                       "IBI for unknown device %#x\n", ibi->addr);
+               /*
+                * Either an IBI received just before IBI's were disabled, or
+                * the controller is broken. Assume the former.
+                */
+               dev_dbg(&hci->master.dev, "IBI when not enabled at address %#x\n", ibi->addr);
                return true;
        }