[PATCH V2 1/7] i3c: mipi-i3c-hci: Fix race in i3c_hci_addr_to_dev()
Frank Li
Frank.Li at oss.nxp.com
Mon Jun 8 10:31:03 PDT 2026
On Mon, Jun 08, 2026 at 10:57:54AM +0300, Adrian Hunter wrote:
> 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 at intel.com>
> ---
Reviewed-by: Frank Li <Frank.Li at nxp.com>
>
>
> Changes in V2:
>
> Factor out __i3c_hci_disable_ibi() to facilitate also clearing
> ibi_devs[dat_idx] upon IBI free, and update commit message
> accordingly.
> Demote a message in PIO and DMA IBI handling, and update commit
> message accordingly.
>
>
> drivers/i3c/master/mipi-i3c-hci/core.c | 37 ++++++++++++++++++++++++--
> drivers/i3c/master/mipi-i3c-hci/dma.c | 7 +++--
> drivers/i3c/master/mipi-i3c-hci/hci.h | 1 +
> drivers/i3c/master/mipi-i3c-hci/ibi.h | 13 +--------
> drivers/i3c/master/mipi-i3c-hci/pio.c | 7 +++--
> 5 files changed, 47 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> index 53797841b63f..1e1f05aff092 100644
> --- 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 87622d6f817e..0672ed1132f8 100644
> --- 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 41d31a53abd3..b3d9803b1968 100644
> --- 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 e1f98e264da0..073ca67b7d04 100644
> --- 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 b5ae1cfaa9e0..ff2657ee220b 100644
> --- 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;
> }
>
> --
> 2.51.0
>
More information about the linux-i3c
mailing list