[PATCH V3 00/14] i3c: Support IBI-based system wakeup

[PATCH V3 00/14] i3c: Support IBI-based system wakeup

[Adrian Hunter]

Hi

Intel LPSS I3C controllers support up to two I3C busses and can wake the
system from an In-Band Interrupt (IBI) via a PCI PME.  Today that wakeup
capability lives only at the PCI function, with no way to express which
I3C device is actually responsible for waking the system, and no way for
user space to enable or disable wakeup on a per-device basis.

This series pushes the wakeup capability down to the individual I3C
devices and then aggregates the resulting wakeup state back up to the PCI
device.

An IBI-capable I3C device on a bus whose controller can wake the system
is marked as wakeup capable, so it can be managed through the standard
device wakeup framework (e.g. via power/wakeup in sysfs).  When such a
device is enabled for wakeup, a wakeup event is reported each time it
queues an IBI.

At suspend time, the mipi-i3c-hci PCI driver aggregates the wakeup
configuration of the I3C devices across its HCI instances (up to two I3C
busses) and enables PCI wakeup (PME) only when at least one attached I3C
device is enabled as a wakeup source and has IBI enabled.  This keeps
the PCI wakeup state in sync with the actual requirements of the devices
on the busses.

The series is organised as follows:

 - Patch 1 fixes a pre-existing recursive acquisition of the bus rwsem
   in i3c_master_register_new_i3c_devs().  The remaining patches add
   work to that same function, so the locking is corrected first.

 - Patches 2-7 fix use-after-free and unlocked accesses of the i3c_device
   desc pointer.  Moving device registration out from under the bus lock
   in patch 1 is what makes it possible to take the bus lock in the
   helpers that run during registration.

 - Patches 8-11 add the generic I3C core support: an ibi_wakeup flag for
   controllers, marking IBI-capable devices as wakeup capable, reporting
   wakeup events on IBIs, a helper to query whether any device on a bus
   has both wakeup and IBI enabled, and a fix to reject IBI requests
   from devices that do not advertise IBI capability.

 - Patches 12-14 wire this up for the mipi-i3c-hci driver: propagate the
   aggregated I3C wakeup requirements to the PCI function, factor out
   i3c_hci_sysdev() for the shared device lookup, and advertise IBI
   wakeup capability when the underlying system device can wake the
   system.

Note, since the PCI wakeup state is now derived from the wakeup
configuration of the attached I3C devices, the PCI device's
power/wakeup sysfs attribute no longer provides independent wakeup
control.


Changes in V3:

    Added 6 patches (2-7) that fix use-after-free and unlocked accesses
    of the i3c_device desc pointer.  Moving device registration out from
    under the bus lock in patch 1 is what allows that pointer to be
    protected in the helpers that run during registration.

    i3c: master: Fix recursive locking during device registration
	Added Cc: stable@vger.kernel.org

    i3c: master: Add helper to query bus wakeup requirements
	Skip the master device explicitly.  Noted in the kernel-doc that
	wakeup enablement is user space policy, so the helper is meant to
	be called from a system suspend callback.

    Added Frank Li's Reviewed-by tags.

    Rebased onto i3c/next.


Changes in V2:

    Dropped the RFC tag.

    i3c: master: Fix recursive locking during device registration
	New patch

    i3c: master: Support IBI-based wakeup capability
	Dropped the redundant #include <linux/pm_wakeup.h>.  That header
	must not be included directly, and linux/device.h, which is
	already included, provides device_set_wakeup_capable().

    i3c: master: Add helper to query bus wakeup requirements
	i3c_master_any_wakeup_enabled() now also requires the device to
	have IBI enabled, not just system wakeup enabled, so that a
	device with no active IBI request does not keep PCI PME enabled.
	desc->ibi_lock is taken while checking.  The commit message and
	kernel-doc are updated to match.

    Rebased onto i3c/next.


Adrian Hunter (14):
      i3c: master: Fix recursive locking during device registration
      i3c: Fix unlocked dereference of dev->desc in i3c_device_get_supported_xfer_mode()
      i3c: master: Do not treat master device as a duplicate target
      i3c: master: Fix use-after-free of master->this
      i3c: Make dev->desc locking assumptions explicit
      i3c: master: Fix potential UAF in i3c_device_uevent()
      i3c: master: Fix potential UAF in i3c_device_match()
      i3c: master: Support IBI-based wakeup capability
      i3c: master: Report wakeup events for IBIs
      i3c: master: Add helper to query bus wakeup requirements
      i3c: master: Reject IBI requests from non-IBI-capable devices
      i3c: mipi-i3c-hci-pci: Propagate I3C wakeup requirements to PCI
      i3c: mipi-i3c-hci: Factor out i3c_hci_sysdev()
      i3c: mipi-i3c-hci: Advertise IBI wakeup capability

 drivers/i3c/device.c                               |  13 +--
 drivers/i3c/internals.h                            |   5 +
 drivers/i3c/master.c                               | 120 ++++++++++++++++-----
 drivers/i3c/master/mipi-i3c-hci/core.c             |  15 +++
 drivers/i3c/master/mipi-i3c-hci/dma.c              |  15 +--
 drivers/i3c/master/mipi-i3c-hci/hci.h              |   2 +
 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c |  23 +++-
 include/linux/i3c/master.h                         |   5 +
 8 files changed, 150 insertions(+), 48 deletions(-)


Regards
Adrian

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH V3 01/14] i3c: master: Fix recursive locking during device registration

[Adrian Hunter]

i3c_master_register_new_i3c_devs() registers newly discovered devices
while holding i3c_bus_normaluse_lock(), a down_read().  device_register()
can immediately probe the device, and probe callbacks typically invoke
I3C helpers that take i3c_bus_normaluse_lock() again, leading to a
recursive acquisition of the same rwsem.  rwsems do not support recursive
read locking and can deadlock when a writer is waiting.  See the
"Recursive read locks" section of Documentation/locking/lockdep-design.rst.

For example, with Intel LPSS I3C, LOCKDEP generates a WARNING like:
  # echo intel-lpss-i3c.0 > /sys/bus/platform/drivers/mipi-i3c-hci/unbind
  # echo intel-lpss-i3c.0 > /sys/bus/platform/drivers/mipi-i3c-hci/bind
  WARNING: possible recursive locking detected
  kworker/5:1/94 is trying to acquire lock:
  ffff88811c810d78 (&i3cbus->lock){++++}-{4:4}, at: i3c_device_match_id+0x45/0x370
  but task is already holding lock:
  ffff88811c810d78 (&i3cbus->lock){++++}-{4:4}, at: i3c_master_reg_work_fn+0x21/0x5f0

Fix this by separating device creation from device registration.
Populate desc->dev under the maintenance lock, collect the devices that
still need registration into a local list, then release the lock before
calling device_register().  Finally retake the lock and clean up any
devices that failed to register.

Use the maintenance lock rather than the normal-use lock while adding
device objects.  A write-side maintenance lock prevents readers from
observing a partially initialized desc->dev during initial device
population, or desc->dev disappearing if registration fails.

The local list requires a list node, so add a list node member to struct
i3c_device.

Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
Cc: stable@vger.kernel.org
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V3:

	Added Cc: stable@vger.kernel.org

Changes in V2:

	New patch


 drivers/i3c/master.c       | 45 ++++++++++++++++++++++++++++----------
 include/linux/i3c/master.h |  2 ++
 2 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index f485b98805cf..d2fb1a110521 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -2069,12 +2069,21 @@ static int i3c_master_early_i3c_dev_add(struct i3c_master_controller *master,
 static void
 i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
 {
+	struct i3c_device *i3cdev, *tmp;
 	struct i3c_dev_desc *desc;
+	LIST_HEAD(i3c_unreg_devs);
 	int ret;
 
 	if (!master->init_done)
 		return;
 
+	i3c_bus_maintenance_lock(&master->bus);
+
+	if (master->shutting_down) {
+		i3c_bus_maintenance_unlock(&master->bus);
+		return;
+	}
+
 	i3c_bus_for_each_i3cdev(&master->bus, desc) {
 		if (desc->dev || !desc->info.dyn_addr || desc == master->this)
 			continue;
@@ -2104,25 +2113,37 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
 		if (desc->boardinfo)
 			device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);
 
-		ret = device_register(&desc->dev->dev);
-		if (ret) {
-			dev_err(&master->dev,
-				"Failed to add I3C device (err = %d)\n", ret);
-			desc->dev->desc = NULL;
-			put_device(&desc->dev->dev);
-			desc->dev = NULL;
-		}
+		list_add_tail(&desc->dev->node, &i3c_unreg_devs);
+	}
+
+	i3c_bus_maintenance_unlock(&master->bus);
+
+	list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
+		ret = device_register(&i3cdev->dev);
+		if (ret)
+			dev_err(&master->dev, "Failed to add I3C device (err = %d)\n", ret);
+		else
+			list_del_init(&i3cdev->node);
+	}
+
+	i3c_bus_maintenance_lock(&master->bus);
+
+	list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
+		list_del(&i3cdev->node);
+		desc = i3cdev->desc;
+		i3cdev->desc = NULL;
+		put_device(&i3cdev->dev);
+		desc->dev = NULL;
 	}
+
+	i3c_bus_maintenance_unlock(&master->bus);
 }
 
 static void i3c_master_reg_work_fn(struct work_struct *work)
 {
 	struct i3c_master_controller *master = container_of(work, typeof(*master), reg_work);
 
-	i3c_bus_normaluse_lock(&master->bus);
-	if (!master->shutting_down)
-		i3c_master_register_new_i3c_devs(master);
-	i3c_bus_normaluse_unlock(&master->bus);
+	i3c_master_register_new_i3c_devs(master);
 }
 
 /**
diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
index 2dc139a217bf..2b96c4ea75fb 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -238,6 +238,7 @@ struct i3c_dev_desc {
  *	  every time the I3C device is rediscovered with a different dynamic
  *	  address assigned
  * @bus: I3C bus this device is attached to
+ * @node: unregistered device list node
  *
  * I3C device object exposed to I3C device drivers. The takes care of linking
  * this object to the relevant &struct_i3c_dev_desc one.
@@ -248,6 +249,7 @@ struct i3c_device {
 	struct device dev;
 	struct i3c_dev_desc *desc;
 	struct i3c_bus *bus;
+	struct list_head node;
 };
 
 /*
-- 
2.53.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH V3 02/14] i3c: Fix unlocked dereference of dev->desc in i3c_device_get_supported_xfer_mode()

[Adrian Hunter]

i3c_device_get_supported_xfer_mode() uses dev->desc to obtain the
master controller.  However, dev->desc must not be dereferenced unless
bus->lock is held, and this function does not take that lock.

The function only needs access to the master controller associated with
the device's bus.  Use dev->bus instead, which is always valid for the
lifetime of the device and does not require dereferencing dev->desc.

Fixes: 256a21743d91 ("i3c: Add HDR API support")
Cc: stable@vger.kernel.org
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V3:

	New patch


 drivers/i3c/device.c    | 2 +-
 drivers/i3c/internals.h | 5 +++++
 drivers/i3c/master.c    | 6 ------
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
index 101eaa77de68..a3778282e84c 100644
--- a/drivers/i3c/device.c
+++ b/drivers/i3c/device.c
@@ -309,7 +309,7 @@ EXPORT_SYMBOL_GPL(i3c_device_match_id);
  */
 u32 i3c_device_get_supported_xfer_mode(struct i3c_device *dev)
 {
-	return i3c_dev_get_master(dev->desc)->this->info.hdr_cap | BIT(I3C_SDR);
+	return i3c_bus_to_i3c_master(dev->bus)->this->info.hdr_cap | BIT(I3C_SDR);
 }
 EXPORT_SYMBOL_GPL(i3c_device_get_supported_xfer_mode);
 
diff --git a/drivers/i3c/internals.h b/drivers/i3c/internals.h
index 0f1f3f766623..86a36b951e0d 100644
--- a/drivers/i3c/internals.h
+++ b/drivers/i3c/internals.h
@@ -72,4 +72,9 @@ static inline void i3c_readl_fifo(const void __iomem *addr, void *buf,
 	}
 }
 
+static inline struct i3c_master_controller *i3c_bus_to_i3c_master(struct i3c_bus *i3cbus)
+{
+	return container_of(i3cbus, struct i3c_master_controller, bus);
+}
+
 #endif /* I3C_INTERNAL_H */
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index d2fb1a110521..c7bb52b71d88 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -102,12 +102,6 @@ void i3c_bus_normaluse_unlock(struct i3c_bus *bus)
 	up_read(&bus->lock);
 }
 
-static struct i3c_master_controller *
-i3c_bus_to_i3c_master(struct i3c_bus *i3cbus)
-{
-	return container_of(i3cbus, struct i3c_master_controller, bus);
-}
-
 static struct i3c_master_controller *dev_to_i3cmaster(struct device *dev)
 {
 	return container_of(dev, struct i3c_master_controller, dev);
-- 
2.53.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH V3 03/14] i3c: master: Do not treat master device as a duplicate target

[Adrian Hunter]

i3c_master_search_i3c_dev_duplicate() searches the bus for another I3C
device with the same PID as the reference device.  The search can match
master->this, causing the controller itself to be returned as a
duplicate.

Since the controller is not a target device, it cannot be a duplicate of
one.  Exclude master->this from matching so that the function only
returns real duplicate target devices.

Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
Cc: stable@vger.kernel.org
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V3:

	New patch


 drivers/i3c/master.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index c7bb52b71d88..abb582645a2e 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -2545,7 +2545,8 @@ i3c_master_search_i3c_dev_duplicate(struct i3c_dev_desc *refdev)
 
 	i3c_bus_for_each_i3cdev(&master->bus, i3cdev) {
 		if (i3cdev != refdev && i3cdev->info.pid &&
-		    i3cdev->info.pid == refdev->info.pid)
+		    i3cdev->info.pid == refdev->info.pid &&
+		    i3cdev != master->this)
 			return i3cdev;
 	}
 
-- 
2.53.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH V3 04/14] i3c: master: Fix use-after-free of master->this

[Adrian Hunter]

sysfs attribute callbacks for the master controller device dereference
master->this.  However, master->this is currently freed in
i3c_master_detach_free_devs(), before the master device itself is
released.

As a result, sysfs accesses can dereference a freed master->this
pointer, leading to a use-after-free.

Keep master->this alive until i3c_masterdev_release(), where all users
of the master device have gone away and the associated sysfs state is
being torn down.  Do not free master->this as part of the normal device
detach path.

Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
Cc: stable@vger.kernel.org
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V3:

	New patch


 drivers/i3c/master.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index abb582645a2e..4839c1c186eb 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -842,6 +842,11 @@ static struct attribute *i3c_masterdev_attrs[] = {
 };
 ATTRIBUTE_GROUPS(i3c_masterdev);
 
+static void i3c_master_free_i3c_dev(struct i3c_dev_desc *dev)
+{
+	kfree(dev);
+}
+
 static void i3c_masterdev_release(struct device *dev)
 {
 	struct i3c_master_controller *master = dev_to_i3cmaster(dev);
@@ -854,6 +859,8 @@ static void i3c_masterdev_release(struct device *dev)
 	i3c_bus_cleanup(bus);
 
 	fwnode_handle_put(dev->fwnode);
+
+	i3c_master_free_i3c_dev(master->this);
 }
 
 static const struct device_type i3c_masterdev_type = {
@@ -1125,11 +1132,6 @@ static void i3c_device_release(struct device *dev)
 	kfree(i3cdev);
 }
 
-static void i3c_master_free_i3c_dev(struct i3c_dev_desc *dev)
-{
-	kfree(dev);
-}
-
 static struct i3c_dev_desc *
 i3c_master_alloc_i3c_dev(struct i3c_master_controller *master,
 			 const struct i3c_device_info *info)
@@ -2286,7 +2288,8 @@ static void i3c_master_detach_free_devs(struct i3c_master_controller *master)
 					i3cdev->boardinfo->init_dyn_addr,
 					I3C_ADDR_SLOT_FREE);
 
-		i3c_master_free_i3c_dev(i3cdev);
+		if (i3cdev != master->this)
+			i3c_master_free_i3c_dev(i3cdev);
 	}
 
 	list_for_each_entry_safe(i2cdev, i2ctmp, &master->bus.devs.i2c,
-- 
2.53.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH V3 05/14] i3c: Make dev->desc locking assumptions explicit

[Adrian Hunter]

i3c_device_get_info() takes the bus normal-use lock before accessing
dev->desc. Under that lock, the descriptor pointer is guaranteed to be
valid for the duration of the access.

Remove the unnecessary NULL check on dev->desc so the code more clearly
reflects the locking rules and expected descriptor lifetime.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V3:

	New patch


 drivers/i3c/device.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
index a3778282e84c..5e6df6de0283 100644
--- a/drivers/i3c/device.c
+++ b/drivers/i3c/device.c
@@ -101,8 +101,7 @@ void i3c_device_get_info(const struct i3c_device *dev,
 		return;
 
 	i3c_bus_normaluse_lock(dev->bus);
-	if (dev->desc)
-		*info = dev->desc->info;
+	*info = dev->desc->info;
 	i3c_bus_normaluse_unlock(dev->bus);
 }
 EXPORT_SYMBOL_GPL(i3c_device_get_info);
-- 
2.53.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH V3 06/14] i3c: master: Fix potential UAF in i3c_device_uevent()

[Adrian Hunter]

i3c_device_uevent() dereferences i3cdev->desc without holding the bus
normal-use lock.  Since the descriptor pointer can be replaced
concurrently, including when a uevent is generated from sysfs, this can
result in dereferencing a stale descriptor and lead to a use-after-free.

Use i3c_device_get_info() instead, which protects access to the
descriptor with the normal-use lock.

Commit 6cf7b65f7029 ("i3c: Use i3cdev->desc->info instead of calling
i3c_device_get_info() to avoid deadlock") replaced the accessor with a
direct descriptor dereference because i3c_device_get_info() would
recursively acquire bus->lock during device registration.

This change depends on "i3c: master: Fix recursive locking during device
registration", which moves device registration out from under bus->lock
and removes the possibility of that deadlock.  Without that change,
restoring the i3c_device_get_info() call would reintroduce the deadlock.

Fixes: 6cf7b65f7029 ("i3c: Use i3cdev->desc->info instead of calling i3c_device_get_info() to avoid deadlock")
Cc: stable@vger.kernel.org # requires "i3c: master: Fix recursive locking during device registration"
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V3:

	New patch


 drivers/i3c/master.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 4839c1c186eb..947ab3c681d5 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -316,8 +316,7 @@ static int i3c_device_uevent(const struct device *dev, struct kobj_uevent_env *e
 	struct i3c_device_info devinfo;
 	u16 manuf, part, ext;
 
-	if (i3cdev->desc)
-		devinfo = i3cdev->desc->info;
+	i3c_device_get_info(i3cdev, &devinfo);
 	manuf = I3C_PID_MANUF_ID(devinfo.pid);
 	part = I3C_PID_PART_ID(devinfo.pid);
 	ext = I3C_PID_EXTRA_INFO(devinfo.pid);
-- 
2.53.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH V3 07/14] i3c: master: Fix potential UAF in i3c_device_match()

[Adrian Hunter]

i3c_device_match() dereferences i3cdev->desc without holding the bus
normal-use lock.  Since the descriptor pointer can be replaced
concurrently, the dereference can race with descriptor replacement and
result in a use-after-free.

Protect access to i3cdev->desc with the normal-use lock.  While the lock
is held, the descriptor is guaranteed to remain valid, so the NULL check
is also unnecessary and can be removed.

This change depends on "i3c: master: Fix recursive locking during device
registration".  Prior to that change, taking the normal-use lock in
i3c_device_match() could recurse on bus->lock during device
registration.

This fixes "i3c: master: match I3C device through DT and ACPI".

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V3:

	New patch


 drivers/i3c/master.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 947ab3c681d5..e7ea41007889 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -347,8 +347,10 @@ static int i3c_device_match(struct device *dev, const struct device_driver *drv)
 	i3cdev = dev_to_i3cdev(dev);
 	i3cdrv = drv_to_i3cdrv(drv);
 
-	if (i3cdev->desc && i3cdev->desc->boardinfo)
+	i3c_bus_normaluse_lock(i3cdev->bus);
+	if (i3cdev->desc->boardinfo)
 		static_addr_method = i3cdev->desc->boardinfo->static_addr_method;
+	i3c_bus_normaluse_unlock(i3cdev->bus);
 
 	/*
 	 * SETAASA-based devices need not always have a matching ID since
-- 
2.53.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH V3 08/14] i3c: master: Support IBI-based wakeup capability

[Adrian Hunter]

An I3C controller acts as a bus controller for one or more I3C devices.
If the controller can wake the system in response to an In-Band
Interrupt (IBI), then any device on that bus that is capable of
generating IBIs can potentially be used as a wakeup source.

Add an ibi_wakeup flag to struct i3c_master_controller so controller
drivers can advertise support for IBI-based wakeup.

If set, mark IBI-capable I3C devices as wakeup capable when they are
registered, allowing wakeup management through the standard device
wakeup framework.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---


Changes in V3:

	None

Changes in V2:

	Dropped the redundant #include <linux/pm_wakeup.h>.  That header
	must not be included directly, and linux/device.h, which is
	already included, provides device_set_wakeup_capable().


 drivers/i3c/master.c       | 7 +++++++
 include/linux/i3c/master.h | 2 ++
 2 files changed, 9 insertions(+)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index e7ea41007889..e6b320da475e 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -2110,6 +2110,13 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
 		if (desc->boardinfo)
 			device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);
 
+		/*
+		 * In the case of IBI wakeup, any IBI-capable device can
+		 * wakeup.
+		 */
+		if (master->ibi_wakeup && (desc->info.bcr & I3C_BCR_IBI_REQ_CAP))
+			device_set_wakeup_capable(&desc->dev->dev, true);
+
 		list_add_tail(&desc->dev->node, &i3c_unreg_devs);
 	}
 
diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
index 2b96c4ea75fb..16394aca7230 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -523,6 +523,7 @@ struct i3c_master_controller_ops {
  * @hotjoin: true if the master support hotjoin
  * @rpm_allowed: true if Runtime PM allowed
  * @rpm_ibi_allowed: true if IBI and Hot-Join allowed while runtime suspended
+ * @ibi_wakeup: IBI can wakeup the system
  * @shutting_down: set to true when master begins shutdown or unregister
  * @boardinfo.i3c: list of I3C  boardinfo objects
  * @boardinfo.i2c: list of I2C boardinfo objects
@@ -562,6 +563,7 @@ struct i3c_master_controller {
 	unsigned int hotjoin: 1;
 	unsigned int rpm_allowed: 1;
 	unsigned int rpm_ibi_allowed: 1;
+	unsigned int ibi_wakeup: 1;
 	bool shutting_down;
 	struct {
 		struct list_head i3c;
-- 
2.53.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH V3 09/14] i3c: master: Report wakeup events for IBIs

[Adrian Hunter]

An I3C device configured as a wakeup source can wake the system by
generating an In-Band Interrupt (IBI).

When an IBI is queued for processing, record a wakeup event for the
device if wakeup is enabled.  Use a 100 ms processing interval to give
the I3C device driver time to process the IBI.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V2 and V3:

	None


 drivers/i3c/master.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index e6b320da475e..8c9e62e6f146 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -3385,6 +3385,9 @@ static void i3c_master_unregister_i3c_devs(struct i3c_master_controller *master)
 	}
 }
 
+/* Approximate time for IBI handler to run */
+#define I3C_WAKEUP_PROCESSING_TIME_MS 100
+
 /**
  * i3c_master_queue_ibi() - Queue an IBI
  * @dev: the device this IBI is coming from
@@ -3398,6 +3401,9 @@ void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot)
 	if (!dev->ibi || !slot)
 		return;
 
+	if (device_may_wakeup(&dev->dev->dev))
+		pm_wakeup_event(&dev->dev->dev, I3C_WAKEUP_PROCESSING_TIME_MS);
+
 	atomic_inc(&dev->ibi->pending_ibis);
 	queue_work(dev->ibi->wq, &slot->work);
 }
-- 
2.53.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH V3 10/14] i3c: master: Add helper to query bus wakeup requirements

[Adrian Hunter]

Add i3c_master_any_wakeup_enabled(), which iterates over the devices on
an I3C bus and reports whether any of them are enabled for system
wakeup and have IBI enabled.

Controller drivers can use this helper to determine whether wakeup
support must remain available while the system is suspended.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V3:

	Skip the master device explicitly.  Added kernel-doc noting
	that wakeup enablement is user space policy, so the helper is
	meant to be called from a system suspend callback.

Changes in V2:

	i3c_master_any_wakeup_enabled() now also requires the device to
	have IBI enabled, not just system wakeup enabled, so that a
	device with no active IBI request does not keep PCI PME enabled.
	desc->ibi_lock is taken while checking.  The commit message and
	kernel-doc are updated to match.


 drivers/i3c/master.c       | 35 +++++++++++++++++++++++++++++++++++
 include/linux/i3c/master.h |  1 +
 2 files changed, 36 insertions(+)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 8c9e62e6f146..baf4769f0512 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -2150,6 +2150,41 @@ static void i3c_master_reg_work_fn(struct work_struct *work)
 	i3c_master_register_new_i3c_devs(master);
 }
 
+/**
+ * i3c_master_any_wakeup_enabled() - check if any device can wake the system
+ * @master: I3C master controller
+ *
+ * Iterate over devices on the bus and return true if any device has
+ * system wakeup enabled and IBI enabled.
+ *
+ * Whether a device is enabled for system wakeup is user space policy,
+ * settable at any time through the device's power/wakeup sysfs attribute,
+ * so the answer is only stable once user space is frozen.  Call this from
+ * a system suspend callback.
+ *
+ * Return: true if any device may wake the system via IBI, false otherwise.
+ */
+bool i3c_master_any_wakeup_enabled(struct i3c_master_controller *master)
+{
+	struct i3c_dev_desc *desc;
+	bool wakeup = false;
+
+	i3c_bus_normaluse_lock(&master->bus);
+	i3c_bus_for_each_i3cdev(&master->bus, desc) {
+		if (!desc->dev || desc == master->this || !device_may_wakeup(&desc->dev->dev))
+			continue;
+		guard(mutex)(&desc->ibi_lock);
+		if (desc->ibi && desc->ibi->enabled) {
+			wakeup = true;
+			break;
+		}
+	}
+	i3c_bus_normaluse_unlock(&master->bus);
+
+	return wakeup;
+}
+EXPORT_SYMBOL_GPL(i3c_master_any_wakeup_enabled);
+
 /**
  * i3c_master_dma_map_single() - Map buffer for single DMA transfer
  * @dev: device object of a device doing DMA
diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
index 16394aca7230..a579a2b3feeb 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -764,6 +764,7 @@ void i3c_generic_ibi_recycle_slot(struct i3c_generic_ibi_pool *pool,
 				  struct i3c_ibi_slot *slot);
 
 void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot);
+bool i3c_master_any_wakeup_enabled(struct i3c_master_controller *master);
 
 struct i3c_ibi_slot *i3c_master_get_free_ibi_slot(struct i3c_dev_desc *dev);
 
-- 
2.53.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH V3 11/14] i3c: master: Reject IBI requests from non-IBI-capable devices

[Adrian Hunter]

i3c_device_request_ibi() does not verify that a device advertises IBI
support before attempting to set up IBI handling.

Add a check for I3C_BCR_IBI_REQ_CAP and fail with -EOPNOTSUPP when IBI
support is not reported by the device.  This keeps IBI setup consistent
with other IBI-related functionality, such as exposing wakeup capability
only for IBI-capable devices.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---


Changes in V3:

	Added Frank's Rev-by tag

Changes in V2:

	None


 drivers/i3c/device.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
index 5e6df6de0283..f1ba363b22a1 100644
--- a/drivers/i3c/device.c
+++ b/drivers/i3c/device.c
@@ -204,12 +204,14 @@ int i3c_device_request_ibi(struct i3c_device *dev,
 		return ret;
 
 	i3c_bus_normaluse_lock(dev->bus);
-	if (dev->desc) {
+	if (!dev->desc) {
+		ret = -ENOENT;
+	} else if (!(dev->desc->info.bcr & I3C_BCR_IBI_REQ_CAP)) {
+		ret = -EOPNOTSUPP;
+	} else {
 		mutex_lock(&dev->desc->ibi_lock);
 		ret = i3c_dev_request_ibi_locked(dev->desc, req);
 		mutex_unlock(&dev->desc->ibi_lock);
-	} else {
-		ret = -ENOENT;
 	}
 	i3c_bus_normaluse_unlock(dev->bus);
 
-- 
2.53.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH V3 12/14] i3c: mipi-i3c-hci-pci: Propagate I3C wakeup requirements to PCI

[Adrian Hunter]

Keep the PCI wakeup state aligned with the wakeup requirements of the
devices served by the controller(s).

The PCI function is the wakeup source for HCI instances exposed beneath
it.  However, wakeup is only needed when at least one attached I3C device
is enabled as a wakeup source.

During suspend, check whether any HCI instance has a wakeup-enabled I3C
device and enable wakeup for the PCI function only in that case.
Otherwise leave PCI wakeup disabled.

Note, the suspend callback is used for both system and runtime suspend.
Although this change may update the PCI wakeup state during runtime
suspend, it does so only when the required wakeup state changes.
Moreover, PCI wakeup-capable devices already have PME wakeup armed for
runtime suspend, so changing the wakeup-enabled state does not affect
runtime PM wakeup behavior.

Note also, since the PCI wakeup state is derived from the wakeup
configuration of the attached I3C devices, the PCI device power/wakeup
sysfs attribute no longer provides independent wakeup control.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V2 and V3:

	None


 .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 23 +++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
index 5a9e2a43eff8..2b3bf6fa74f2 100644
--- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
+++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
@@ -265,6 +265,8 @@ static bool mipi_i3c_hci_pci_is_operational(struct device *dev, bool update)
 struct mipi_i3c_hci_pci_pm_data {
 	struct device *dev[INST_MAX];
 	int dev_cnt;
+	bool can_wakeup;
+	bool may_wakeup;
 };
 
 static bool mipi_i3c_hci_pci_is_mfd(struct device *dev)
@@ -272,6 +274,13 @@ static bool mipi_i3c_hci_pci_is_mfd(struct device *dev)
 	return dev_is_platform(dev) && mfd_get_cell(to_platform_device(dev));
 }
 
+static bool mipi_i3c_hci_pci_any_wakeup_enabled(struct device *dev)
+{
+	struct i3c_hci *hci = dev_get_drvdata(dev);
+
+	return i3c_master_any_wakeup_enabled(&hci->master);
+}
+
 static int mipi_i3c_hci_pci_suspend_instance(struct device *dev, void *data)
 {
 	struct mipi_i3c_hci_pci_pm_data *pm_data = data;
@@ -287,6 +296,9 @@ static int mipi_i3c_hci_pci_suspend_instance(struct device *dev, void *data)
 
 	pm_data->dev[pm_data->dev_cnt++] = dev;
 
+	if (pm_data->can_wakeup && mipi_i3c_hci_pci_any_wakeup_enabled(dev))
+		pm_data->may_wakeup = true;
+
 	return 0;
 }
 
@@ -317,12 +329,19 @@ static int mipi_i3c_hci_pci_suspend(struct device *dev)
 	if (!hci->info->control_instance_pm)
 		return 0;
 
+	pm_data.can_wakeup = device_can_wakeup(dev);
+
 	ret = device_for_each_child_reverse(dev, &pm_data, mipi_i3c_hci_pci_suspend_instance);
-	if (ret)
+	if (ret) {
 		for (int i = 0; i < pm_data.dev_cnt; i++)
 			i3c_hci_rpm_resume(pm_data.dev[i]);
+		return ret;
+	}
 
-	return ret;
+	if (device_may_wakeup(dev) != pm_data.may_wakeup)
+		device_set_wakeup_enable(dev, pm_data.may_wakeup);
+
+	return 0;
 }
 
 static int mipi_i3c_hci_pci_resume(struct device *dev)
-- 
2.53.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH V3 13/14] i3c: mipi-i3c-hci: Factor out i3c_hci_sysdev()

[Adrian Hunter]

The MIPI I3C HCI driver needs to identify the underlying system device
used for DMA mapping and PM operations. The logic for determining that
device is currently embedded in the DMA implementation.

Factor this code out into i3c_hci_sysdev() so it can be shared by other
parts of the driver and keep the device-selection logic in one place.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V2 and V3:

	None


 drivers/i3c/master/mipi-i3c-hci/core.c | 12 ++++++++++++
 drivers/i3c/master/mipi-i3c-hci/dma.c  | 15 +--------------
 drivers/i3c/master/mipi-i3c-hci/hci.h  |  2 ++
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
index cfe9b5390b56..0212c9e984cf 100644
--- a/drivers/i3c/master/mipi-i3c-hci/core.c
+++ b/drivers/i3c/master/mipi-i3c-hci/core.c
@@ -15,6 +15,7 @@
 #include <linux/interrupt.h>
 #include <linux/iopoll.h>
 #include <linux/module.h>
+#include <linux/pci.h>
 #include <linux/platform_data/mipi-i3c-hci.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
@@ -117,6 +118,17 @@ static inline struct i3c_hci *to_i3c_hci(struct i3c_master_controller *m)
 	return container_of(m, struct i3c_hci, master);
 }
 
+/*
+ * Determine the device that does PM / DMA and has IOMMU setup done for it in
+ * case of enabled IOMMU (for use with the DMA API).
+ * Such device is either "mipi-i3c-hci" platform device (OF/ACPI enumeration)
+ * parent or grandparent (PCI enumeration).
+ */
+struct device *i3c_hci_sysdev(struct device *dev)
+{
+	return dev->parent && dev_is_pci(dev->parent) ? dev->parent : dev;
+}
+
 static void i3c_hci_set_master_dyn_addr(struct i3c_hci *hci)
 {
 	reg_write(MASTER_DEVICE_ADDR,
diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
index 0672ed1132f8..7c2b20474130 100644
--- a/drivers/i3c/master/mipi-i3c-hci/dma.c
+++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
@@ -15,7 +15,6 @@
 #include <linux/errno.h>
 #include <linux/i3c/master.h>
 #include <linux/io.h>
-#include <linux/pci.h>
 
 #include "hci.h"
 #include "cmd.h"
@@ -301,23 +300,11 @@ static int hci_dma_init(struct i3c_hci *hci)
 {
 	struct hci_rings_data *rings;
 	struct hci_rh_data *rh;
-	struct device *sysdev;
 	u32 regval;
 	unsigned int i, nr_rings, xfers_sz, resps_sz;
 	unsigned int ibi_status_ring_sz, ibi_data_ring_sz;
 	int ret;
 
-	/*
-	 * Set pointer to a physical device that does DMA and has IOMMU setup
-	 * done for it in case of enabled IOMMU and use it with the DMA API.
-	 * Here such device is either
-	 * "mipi-i3c-hci" platform device (OF/ACPI enumeration) parent or
-	 * grandparent (PCI enumeration).
-	 */
-	sysdev = hci->master.dev.parent;
-	if (sysdev->parent && dev_is_pci(sysdev->parent))
-		sysdev = sysdev->parent;
-
 	regval = rhs_reg_read(CONTROL);
 	nr_rings = FIELD_GET(MAX_HEADER_COUNT_CAP, regval);
 	dev_dbg(&hci->master.dev, "%d DMA rings available\n", nr_rings);
@@ -332,7 +319,7 @@ static int hci_dma_init(struct i3c_hci *hci)
 		return -ENOMEM;
 	hci->io_data = rings;
 	rings->total = nr_rings;
-	rings->sysdev = sysdev;
+	rings->sysdev = i3c_hci_sysdev(hci->master.dev.parent);
 
 	for (i = 0; i < rings->total; i++) {
 		u32 offset = rhs_reg_read(RHn_OFFSET(i));
diff --git a/drivers/i3c/master/mipi-i3c-hci/hci.h b/drivers/i3c/master/mipi-i3c-hci/hci.h
index b3d9803b1968..b8d2a3d680f8 100644
--- a/drivers/i3c/master/mipi-i3c-hci/hci.h
+++ b/drivers/i3c/master/mipi-i3c-hci/hci.h
@@ -184,6 +184,8 @@ void amd_set_resp_buf_thld(struct i3c_hci *hci);
 void i3c_hci_sync_irq_inactive(struct i3c_hci *hci);
 int i3c_hci_process_xfer(struct i3c_hci *hci, struct hci_xfer *xfer, int n);
 
+struct device *i3c_hci_sysdev(struct device *dev);
+
 #define DEFAULT_AUTOSUSPEND_DELAY_MS 1000
 
 int i3c_hci_rpm_suspend(struct device *dev);
-- 
2.53.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH V3 14/14] i3c: mipi-i3c-hci: Advertise IBI wakeup capability

[Adrian Hunter]

Set master->ibi_wakeup during probe when the associated system device
advertises wakeup capability, allowing the I3C core to mark IBI-capable
I3C devices as wakeup capable.

Tweak the comment for i3c_hci_sysdev() to mention the new usage.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---


Changes in V3:

	Added Frank's Rev-by tag

Changes in V2:

	None


 drivers/i3c/master/mipi-i3c-hci/core.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
index 0212c9e984cf..5d92cf0645ef 100644
--- a/drivers/i3c/master/mipi-i3c-hci/core.c
+++ b/drivers/i3c/master/mipi-i3c-hci/core.c
@@ -120,9 +120,9 @@ static inline struct i3c_hci *to_i3c_hci(struct i3c_master_controller *m)
 
 /*
  * Determine the device that does PM / DMA and has IOMMU setup done for it in
- * case of enabled IOMMU (for use with the DMA API).
- * Such device is either "mipi-i3c-hci" platform device (OF/ACPI enumeration)
- * parent or grandparent (PCI enumeration).
+ * case of enabled IOMMU (for use with the DMA API). It is also used to check
+ * for wakeup capability. Such device is either "mipi-i3c-hci" platform device
+ * (OF/ACPI enumeration) parent or grandparent (PCI enumeration).
  */
 struct device *i3c_hci_sysdev(struct device *dev)
 {
@@ -1176,6 +1176,9 @@ static int i3c_hci_probe(struct platform_device *pdev)
 	if (hci->quirks & HCI_QUIRK_RPM_IBI_ALLOWED)
 		hci->master.rpm_ibi_allowed = true;
 
+	if (device_can_wakeup(i3c_hci_sysdev(&pdev->dev)))
+		hci->master.ibi_wakeup = true;
+
 	return i3c_master_register(&hci->master, &pdev->dev, &i3c_hci_ops, false);
 }
 
-- 
2.53.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 10/14] i3c: master: Add helper to query bus wakeup requirements

[sashiko-bot]

> Add i3c_master_any_wakeup_enabled(), which iterates over the devices on
> an I3C bus and reports whether any of them are enabled for system
> wakeup and have IBI enabled.
> 
> Controller drivers can use this helper to determine whether wakeup
> support must remain available while the system is suspended.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804133810.184905-1-adrian.hunter@intel.com?part=10


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 08/14] i3c: master: Support IBI-based wakeup capability

[sashiko-bot]

> An I3C controller acts as a bus controller for one or more I3C devices.
> If the controller can wake the system in response to an In-Band
> Interrupt (IBI), then any device on that bus that is capable of
> generating IBIs can potentially be used as a wakeup source.
> 
> Add an ibi_wakeup flag to struct i3c_master_controller so controller
> drivers can advertise support for IBI-based wakeup.
> 
> If set, mark IBI-capable I3C devices as wakeup capable when they are
> registered, allowing wakeup management through the standard device
> wakeup framework.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804133810.184905-1-adrian.hunter@intel.com?part=8


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 04/14] i3c: master: Fix use-after-free of master->this

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] A double free occurs in the error path of `i3c_master_set_info()` because `master->this` is left as a dangling pointer and is subsequently freed again in `i3c_masterdev_release()`.

Pre-existing issues:
- [High] Devres lifetime mismatch allows use-after-free of the master controller struct in `i3c_masterdev_release()`.
--

--- Patch [4]: [PATCH V3 04/14] i3c: master: Fix use-after-free of master->this ---
commit e74d33e25ba87fdc800277aa5702a984d20076ca
Author: Adrian Hunter <adrian.hunter@intel.com>
i3c: master: Fix use-after-free of master->this

This patch delays freeing master->this until i3c_masterdev_release(). By
doing this, it keeps master->this alive until all users of the master
device have gone away and the sysfs state is torn down.

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index abb582645a2ed..4839c1c186eb0 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
[ ... ]
> @@ -842,6 +842,11 @@ static struct attribute *i3c_masterdev_attrs[] = {
>  };
>  ATTRIBUTE_GROUPS(i3c_masterdev);
>  
> +static void i3c_master_free_i3c_dev(struct i3c_dev_desc *dev)
> +{
> +	kfree(dev);
> +}
> +
>  static void i3c_masterdev_release(struct device *dev)
>  {
>  	struct i3c_master_controller *master = dev_to_i3cmaster(dev);
>  	struct i3c_bus *bus = dev_to_i3cbus(dev);

[Severity: High]
This is a pre-existing issue, but does this release function dereference
fields in the master struct after the driver's devres cleanup has already
freed the surrounding memory?

Master controller structures are typically allocated using devm_kzalloc().
When a driver is unbound, devres cleanup frees the master controller
structure. However, if userspace holds an open file descriptor to a sysfs
attribute (like /sys/class/i3c/i3c-0/bcr), the kobject refcount doesn't drop
to zero.

When the sysfs file is later closed, the final put_device() calls this
i3c_masterdev_release() function, which then dereferences fields like
master->wq, bus->devs, and now master->this, leading to a use-after-free.

>  	if (master->wq)
>  		destroy_workqueue(master->wq);
>  
>  	WARN_ON(!list_empty(&bus->devs.i2c) || !list_empty(&bus->devs.i3c));
>  	i3c_bus_cleanup(bus);
>  
>  	fwnode_handle_put(dev->fwnode);
> +
> +	i3c_master_free_i3c_dev(master->this);

[Severity: High]
Does unconditionally freeing master->this here cause a double free on the
error path of i3c_master_set_info()?

If i3c_master_attach_i3c_dev() fails during initialization in
i3c_master_set_info(), the allocated i3cdev is freed via
i3c_master_free_i3c_dev().

However, master->this and master->bus.cur_master were already assigned to
point to i3cdev and are not set back to NULL:

drivers/i3c/master.c:i3c_master_set_info() {
	...
	master->this = i3cdev;
	master->bus.cur_master = master->this;

	ret = i3c_master_attach_i3c_dev(master, i3cdev);
	if (ret)
		goto err_free_dev;
	...
err_free_dev:
	i3c_master_free_i3c_dev(i3cdev);
	return ret;
}

The failure propagates to i3c_master_register(), which calls
put_device(&master->dev) to abort driver registration. This calls
i3c_masterdev_release(), which now unconditionally frees master->this,
leading to a double free of the dangling pointer.

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804133810.184905-1-adrian.hunter@intel.com?part=4

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 03/14] i3c: master: Do not treat master device as a duplicate target

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unregistering an I3C master causes a use-after-free if its sysfs attributes (e.g., `pid`, `bcr`, `dcr`) are read concurrently.
--

--- Patch [3]: [PATCH V3 03/14] i3c: master: Do not treat master device as a duplicate target ---
commit a98a06e576b08be2cd4e71df3fa63ba688a254ca
Author: Adrian Hunter <adrian.hunter@intel.com>

i3c: master: Do not treat master device as a duplicate target

This commit updates i3c_master_search_i3c_dev_duplicate() to exclude the
master controller itself from matching when searching for duplicate target
devices. Since the controller is not a target device, it should not be
returned as a duplicate.

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index c7bb52b71d884..abb582645a2ed 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -2545,7 +2545,8 @@ i3c_master_search_i3c_dev_duplicate(struct i3c_dev_desc *refdev)
>  
>  	i3c_bus_for_each_i3cdev(&master->bus, i3cdev) {
>  		if (i3cdev != refdev && i3cdev->info.pid &&
> -		    i3cdev->info.pid == refdev->info.pid)
> +		    i3cdev->info.pid == refdev->info.pid &&
> +		    i3cdev != master->this)
>  			return i3cdev;
>  	}

[Severity: High]
This is a pre-existing issue, but unregistering an I3C master appears to
cause a use-after-free if its sysfs attributes are read concurrently.

Looking at i3c_master_unregister() in drivers/i3c/master.c:

i3c_master_unregister() {
    ...
    i3c_master_bus_cleanup(master);
    pm_runtime_disable(&master->dev);
    device_unregister(&master->dev);
    ...
}

The call to i3c_master_bus_cleanup() calls i3c_master_detach_free_devs(),
which frees all devices in the list including master->this. 

However, because device_unregister() hasn't been called yet, the sysfs files
remain exposed to userspace.

If userspace reads a sysfs attribute like pid during this window, could this
trigger a use-after-free?

drivers/i3c/master.c:pid_show() {
    ...
    desc = dev_to_i3cdesc(dev);
    ret = sysfs_emit(buf, "%llx\n", desc->info.pid);
    ...
}

Since master->this is already freed but the pointer is not NULL, 
desc->info.pid would dereference the freed memory.

Could device_unregister() be called earlier to prevent the sysfs files 
from being accessed while the cleanup happens?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804133810.184905-1-adrian.hunter@intel.com?part=3

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 05/14] i3c: Make dev->desc locking assumptions explicit

[sashiko-bot]

> i3c_device_get_info() takes the bus normal-use lock before accessing
> dev->desc. Under that lock, the descriptor pointer is guaranteed to be
> valid for the duration of the access.
> 
> Remove the unnecessary NULL check on dev->desc so the code more clearly
> reflects the locking rules and expected descriptor lifetime.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804133810.184905-1-adrian.hunter@intel.com?part=5


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 11/14] i3c: master: Reject IBI requests from non-IBI-capable devices

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The workqueue `ibi->wq` is leaked on the error path if `master->ops->request_ibi(dev, req)` fails.
--

--- Patch [11]: [PATCH V3 11/14] i3c: master: Reject IBI requests from non-IBI-capable devices ---
commit a4ace89706edbc3219fece43aaef4d3c3e594148
Author: Adrian Hunter <adrian.hunter@intel.com>

i3c: master: Reject IBI requests from non-IBI-capable devices

This commit updates i3c_device_request_ibi() to verify that a device
advertises IBI support before attempting to set up IBI handling. If the
device lacks the I3C_BCR_IBI_REQ_CAP capability, it now returns early
with -EOPNOTSUPP.

> diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
> index 5e6df6de02834..f1ba363b22a17 100644
> --- a/drivers/i3c/device.c
> +++ b/drivers/i3c/device.c
> @@ -204,12 +204,14 @@ int i3c_device_request_ibi(struct i3c_device *dev,
>  		return ret;
>  
>  	i3c_bus_normaluse_lock(dev->bus);
> -	if (dev->desc) {
> +	if (!dev->desc) {
> +		ret = -ENOENT;
> +	} else if (!(dev->desc->info.bcr & I3C_BCR_IBI_REQ_CAP)) {
> +		ret = -EOPNOTSUPP;
> +	} else {
>  		mutex_lock(&dev->desc->ibi_lock);
>  		ret = i3c_dev_request_ibi_locked(dev->desc, req);

[Severity: High]
This is a pre-existing issue, but since this patch interacts closely with IBI
request error handling, I noticed a potential resource leak in the underlying
i3c_dev_request_ibi_locked() function.

Does this code leak the newly allocated workqueue if the request fails?

drivers/i3c/master.c:i3c_dev_request_ibi_locked() {
    ...
    ibi->wq = alloc_ordered_workqueue(dev_name(i3cdev_to_dev(dev->dev)), WQ_MEM_RECLAIM);
    ...
    ret = master->ops->request_ibi(dev, req);
    if (ret) {
        kfree(ibi);
        dev->ibi = NULL;
    }
    
    return ret;
}

Should destroy_workqueue(ibi->wq) be called before kfree(ibi) on this error
path to prevent leaking the workqueue if a driver repeatedly fails to request
an IBI?

>  		mutex_unlock(&dev->desc->ibi_lock);
> -	} else {
> -		ret = -ENOENT;
>  	}
>  	i3c_bus_normaluse_unlock(dev->bus);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804133810.184905-1-adrian.hunter@intel.com?part=11

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 13/14] i3c: mipi-i3c-hci: Factor out i3c_hci_sysdev()

[sashiko-bot]

> The MIPI I3C HCI driver needs to identify the underlying system device
> used for DMA mapping and PM operations. The logic for determining that
> device is currently embedded in the DMA implementation.
> 
> Factor this code out into i3c_hci_sysdev() so it can be shared by other
> parts of the driver and keep the device-selection logic in one place.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804133810.184905-1-adrian.hunter@intel.com?part=13


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 14/14] i3c: mipi-i3c-hci: Advertise IBI wakeup capability

[sashiko-bot]

> Set master->ibi_wakeup during probe when the associated system device
> advertises wakeup capability, allowing the I3C core to mark IBI-capable
> I3C devices as wakeup capable.
> 
> Tweak the comment for i3c_hci_sysdev() to mention the new usage.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804133810.184905-1-adrian.hunter@intel.com?part=14


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 01/14] i3c: master: Fix recursive locking during device registration

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-After-Free of `i3cdev->desc` during unlocked driver core callbacks (e.g., `i3c_device_match`) racing with Dynamic Address Assignment (DAA).
--

--- Patch [1]: [PATCH V3 01/14] i3c: master: Fix recursive locking during device registration ---
commit b974ff3a9bff008bbc487a2d9b2d24a4539bff06
Author: Adrian Hunter <adrian.hunter@intel.com>

i3c: master: Fix recursive locking during device registration

This commit fixes recursive locking during i3c device registration by
separating device creation from registration. It populates the device
under the maintenance lock, collects unregistered devices into a list,
and calls device_register() only after releasing the lock.

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c

[ ... ]

> @@ -2104,25 +2113,37 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)

[ ... ]

> +	i3c_bus_maintenance_unlock(&master->bus);
> +
> +	list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
> +		ret = device_register(&i3cdev->dev);

[Severity: High]
This is a pre-existing issue, but does dropping the maintenance lock here
expose driver core callbacks to a use-after-free of i3cdev->desc?

When device_register() runs without the maintenance lock, synchronous driver
core callbacks like i3c_device_match() can locklessly dereference
i3cdev->desc:

drivers/i3c/master.c:i3c_device_match() {
    ...
    if (i3cdev->desc && i3cdev->desc->boardinfo)
        static_addr_method = i3cdev->desc->boardinfo->static_addr_method;
    ...
}

Concurrently, a hot-join event triggering dynamic address assignment (DAA)
could take the maintenance lock and free the old descriptor if it identifies
the device as a duplicate:

drivers/i3c/master.c:__i3c_master_add_i3c_dev_locked() {
    ...
    i3c_master_detach_i3c_dev(olddev);
    i3c_master_free_i3c_dev(olddev);
    ...
}

Could this sequence cause a use-after-free when the unlocked callback
races with DAA freeing the duplicate descriptor?

> +		if (ret)
> +			dev_err(&master->dev, "Failed to add I3C device (err = %d)\n", ret);
> +		else
> +			list_del_init(&i3cdev->node);
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804133810.184905-1-adrian.hunter@intel.com?part=1

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 12/14] i3c: mipi-i3c-hci-pci: Propagate I3C wakeup requirements to PCI

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] NULL pointer dereference in proxy PM callbacks when child device is unbound
--

--- Patch [12]: [PATCH V3 12/14] i3c: mipi-i3c-hci-pci: Propagate I3C wakeup requirements to PCI ---
commit da80b1fc5d708734df314262ab3cf3703c943f07
Author: Adrian Hunter <adrian.hunter@intel.com>
Subject: i3c: mipi-i3c-hci-pci: Propagate I3C wakeup requirements to PCI

This patch aligns the PCI wakeup state with the requirements of the I3C
devices it serves. It checks during suspend if any HCI instance has a
wakeup-enabled I3C device, and arms the PCI function's wakeup accordingly.

> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> index 5a9e2a43eff8d..2b3bf6fa74f29 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
[ ... ]
> @@ -272,6 +274,13 @@ static bool mipi_i3c_hci_pci_is_mfd(struct device *dev)
>  	return dev_is_platform(dev) && mfd_get_cell(to_platform_device(dev));
>  }
>  
> +static bool mipi_i3c_hci_pci_any_wakeup_enabled(struct device *dev)
> +{
> +	struct i3c_hci *hci = dev_get_drvdata(dev);
> +
> +	return i3c_master_any_wakeup_enabled(&hci->master);

[Severity: High]
This is a pre-existing issue, but can this sequence result in a NULL pointer
dereference if the child driver is unbound?

If the mipi-i3c-hci platform driver fails to probe or is manually unbound
via sysfs, dev_get_drvdata(dev) will return NULL.

During system suspend, the PCI parent driver iterates over its child
platform devices in mipi_i3c_hci_pci_suspend(). If the BIOS left the bus
active (or the hardware hung during driver removal and failed to clear
the HC_CONTROL_BUS_ENABLE register), mipi_i3c_hci_pci_is_operational()
will return true.

This causes mipi_i3c_hci_pci_suspend_instance() to call
i3c_hci_rpm_suspend() and this newly added function, both of which will
dereference the NULL hci pointer.

Is it possible to add a NULL check here or in the caller before proceeding
with the suspend operations on the child device?

> +}
> +
>  static int mipi_i3c_hci_pci_suspend_instance(struct device *dev, void *data)
>  {
>  	struct mipi_i3c_hci_pci_pm_data *pm_data = data;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804133810.184905-1-adrian.hunter@intel.com?part=12

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 06/14] i3c: master: Fix potential UAF in i3c_device_uevent()

[sashiko-bot]

> i3c_device_uevent() dereferences i3cdev->desc without holding the bus
> normal-use lock.  Since the descriptor pointer can be replaced
> concurrently, including when a uevent is generated from sysfs, this can
> result in dereferencing a stale descriptor and lead to a use-after-free.
> 
> Use i3c_device_get_info() instead, which protects access to the
> descriptor with the normal-use lock.
> 
> Commit 6cf7b65f7029 ("i3c: Use i3cdev->desc->info instead of calling
> i3c_device_get_info() to avoid deadlock") replaced the accessor with a
> direct descriptor dereference because i3c_device_get_info() would
> recursively acquire bus->lock during device registration.
> 
> This change depends on "i3c: master: Fix recursive locking during device
> registration", which moves device registration out from under bus->lock
> and removes the possibility of that deadlock.  Without that change,
> restoring the i3c_device_get_info() call would reintroduce the deadlock.
> 
> Fixes: 6cf7b65f7029 ("i3c: Use i3cdev->desc->info instead of calling i3c_device_get_info() to avoid deadlock")
> Cc: stable@vger.kernel.org # requires "i3c: master: Fix recursive locking during device registration"
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804133810.184905-1-adrian.hunter@intel.com?part=6


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 02/14] i3c: Fix unlocked dereference of dev->desc in i3c_device_get_supported_xfer_mode()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Critical] The patch transforms a predictable NULL pointer dereference into an exploitable Use-After-Free of `master->this` when an I3C device outlives its master controller's unregistration.

Pre-existing issues:
- [High] Uninitialized stack memory (`devinfo`) is used for driver matching when `i3cdev->desc` is NULL.
- [High] A Time-of-Check to Time-of-Use (TOCTOU) race condition exists when accessing `i3cdev->desc->boardinfo`, leading to a Use-After-Free or NULL pointer dereference.
--

--- Patch [2]: [PATCH V3 02/14] i3c: Fix unlocked dereference of dev->desc in i3c_device_get_supported_xfer_mode() ---
commit 97cfccca08da732124561cbc1aa49b571a84dd8c
Author: Adrian Hunter <adrian.hunter@intel.com>

i3c: Fix unlocked dereference of dev->desc in i3c_device_get_supported_xfer_mode()

This patch modifies i3c_device_get_supported_xfer_mode() to use dev->bus
instead of dev->desc to obtain the master controller. This avoids a potential
unlocked dereference of dev->desc.

> diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
> index 101eaa77de686..a3778282e84c2 100644
> --- a/drivers/i3c/device.c
> +++ b/drivers/i3c/device.c
> @@ -309,7 +309,7 @@ EXPORT_SYMBOL_GPL(i3c_device_match_id);
>   */
>  u32 i3c_device_get_supported_xfer_mode(struct i3c_device *dev)
>  {
> -	return i3c_dev_get_master(dev->desc)->this->info.hdr_cap | BIT(I3C_SDR);
> +	return i3c_bus_to_i3c_master(dev->bus)->this->info.hdr_cap | BIT(I3C_SDR);

[Severity: Critical]
Does this change introduce a use-after-free if the device outlives the master
controller?

When a master is unregistered, i3c_master_unregister() calls
i3c_master_unregister_i3c_devs() to unregister child devices. If a child
device has an elevated refcount (e.g., from an open file descriptor), its
struct i3c_device outlives the unregistration.

i3c_master_bus_cleanup() then calls i3c_master_detach_free_devs(), which
explicitly frees master->this.

If the child device later calls i3c_device_get_supported_xfer_mode(), it
would dereference the explicitly freed master->this pointer. The previous
code safely faulted on a NULL dev->desc, but this new path appears to access
freed memory.

[ ... ]

[Severity: High]
This is a pre-existing issue, but does i3c_device_match() have a TOCTOU race
condition leading to a use-after-free?

drivers/i3c/master.c:i3c_device_match() {
        if (i3cdev->desc && i3cdev->desc->boardinfo)
                static_addr_method = i3cdev->desc->boardinfo->static_addr_method;
}

If i3c_device_match() accesses i3cdev->desc without holding the device lock
during concurrent master unregistration, a context switch could occur right
after i3cdev->desc is checked.

If the unregistration thread then frees the descriptor via kfree() in
i3c_master_bus_cleanup(), the subsequent evaluation of
i3cdev->desc->boardinfo will dereference a freed pointer.

[Severity: High]
This isn't a bug introduced by this patch, but can i3c_device_match_id()
evaluate uninitialized stack memory?

During concurrent driver registration and master unregistration, if
i3c_master_unregister_i3c_devs() sets i3cdev->dev->desc = NULL,
i3c_device_match() might still proceed to call i3c_device_match_id().

drivers/i3c/device.c:i3c_device_match_id() {
        struct i3c_device_info devinfo;
        ...
        i3c_device_get_info(i3cdev, &devinfo);
        
        manuf = I3C_PID_MANUF_ID(devinfo.pid);
}

If i3cdev->desc is NULL, i3c_device_get_info() bails out safely without
populating devinfo, leaving the stack memory completely uninitialized when
it is subsequently read for driver matching.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804133810.184905-1-adrian.hunter@intel.com?part=2

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 09/14] i3c: master: Report wakeup events for IBIs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Unprotected dereference of dev->dev in i3c_master_queue_ibi() leading to NULL pointer dereference or Use-After-Free during teardown.

Pre-existing issues:
- [High] Race condition between hardware IBI interrupts and IBI teardown leading to Use-After-Free of dev->ibi.
--

--- Patch [9]: [PATCH V3 09/14] i3c: master: Report wakeup events for IBIs ---
commit 044b30e8813c466ee0c955f282a081170370a564
Author: Adrian Hunter <adrian.hunter@intel.com>

i3c: master: Report wakeup events for IBIs

When an IBI is queued for processing, this commit records a wakeup event
for the device if wakeup is enabled. It uses a 100 ms processing interval
to give the I3C device driver time to process the IBI.

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index e6b320da475e0..8c9e62e6f1469 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -3398,6 +3401,9 @@ void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot)
>  	if (!dev->ibi || !slot)
>  		return;
>  
> +	if (device_may_wakeup(&dev->dev->dev))
> +		pm_wakeup_event(&dev->dev->dev, I3C_WAKEUP_PROCESSING_TIME_MS);
> +

[Severity: High]
Is it possible for an IBI interrupt to fire concurrently with device
unregistration?

Looking at i3c_master_unregister_i3c_devs(), the teardown path sets
dev->dev to NULL:

drivers/i3c/master.c:i3c_master_unregister_i3c_devs() {
    ...
    i3cdev->dev->desc = NULL;
    put_device(&i3cdev->dev->dev);
    i3cdev->dev = NULL;
    ...
}

If an interrupt occurs during or just after this unregistration, could
dev->dev be NULL when device_may_wakeup() dereferences it here?

>  	atomic_inc(&dev->ibi->pending_ibis);
>  	queue_work(dev->ibi->wq, &slot->work);
>  }

[Severity: High]
This is a pre-existing issue, but is there a race condition between hardware
IBI interrupts and IBI teardown leading to a use-after-free of dev->ibi?

In i3c_dev_disable_ibi_locked(), the code checks the pending_ibis counter:

drivers/i3c/master.c:i3c_dev_disable_ibi_locked() {
    ...
    if (atomic_read(&dev->ibi->pending_ibis))
        wait_for_completion(&dev->ibi->all_ibis_handled);
    ...
}

If a hardirq handler is already executing but has not yet called
i3c_master_queue_ibi() to increment pending_ibis, the teardown path might
assume no IBIs are pending and proceed to free dev->ibi in
i3c_dev_free_ibi_locked().

When the interrupted CPU resumes, wouldn't it execute this atomic_inc() and
queue_work() on the freed dev->ibi pointer since there is no
synchronize_irq() to drain active hardirqs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804133810.184905-1-adrian.hunter@intel.com?part=9

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 07/14] i3c: master: Fix potential UAF in i3c_device_match()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Removal of the NULL check for `i3cdev->desc` introduces a guaranteed NULL pointer dereference in concurrent unregistration paths.
- [High] Unconditional access to `i3cdev->bus` in `i3c_device_match()` causes a Use-After-Free if the parent master controller has been freed.

Pre-existing issues:
- [High] Preexisting use-after-free race condition in AMD SBRMI (`sbrmi_i3c_probe`) accessing `i3cdev->desc` without holding the bus lock.
--

--- Patch [7]: [PATCH V3 07/14] i3c: master: Fix potential UAF in i3c_device_match() ---
commit d088359c0cccb56067d426e92d0a49f24376de53
Author: Adrian Hunter <adrian.hunter@intel.com>

i3c: master: Fix potential UAF in i3c_device_match()

This commit protects access to i3cdev->desc with the bus normal-use lock
in i3c_device_match() to prevent a use-after-free during concurrent
descriptor replacement. It also removes the NULL check for i3cdev->desc,
assuming the lock guarantees validity.

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 947ab3c681d5e..e7ea410078891 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -347,8 +347,10 @@ static int i3c_device_match(struct device *dev, const struct device_driver *drv)
>  	i3cdev = dev_to_i3cdev(dev);
>  	i3cdrv = drv_to_i3cdrv(drv);
>  
> -	if (i3cdev->desc && i3cdev->desc->boardinfo)
> +	i3c_bus_normaluse_lock(i3cdev->bus);

[Severity: High]
Does accessing i3cdev->bus here risk a use-after-free if the parent master
controller has been freed?

During unregistration, i3c_master_unregister() unregisters child devices,
and device_del() drops the child's reference to the parent by calling
put_device(parent). Once unregistration finishes, the parent platform device
unbinds and its devres frees the I3C master struct. 

If a concurrent bus_for_each_dev() iterator (such as from driver registration)
holds a reference to the child device, the child outlives the parent. The
child's i3cdev->bus pointer would then remain pointing to the freed master
memory, leading to a use-after-free when acquiring the lock.

> +	if (i3cdev->desc->boardinfo)

[Severity: High]
Could removing the NULL check for i3cdev->desc lead to a NULL pointer
dereference during concurrent unregistration?

In i3c_master_unregister_i3c_devs(), i3cdev->dev->desc = NULL; is set without
holding any bus locks. 

If a concurrent driver matching thread invokes i3c_device_match(), it can
proceed in parallel because driver_match_device() operates outside
device_lock(). The thread acquires the bus lock, but since unregistration
doesn't hold it, it could read desc as NULL and crash when accessing
i3cdev->desc->boardinfo.

>  		static_addr_method = i3cdev->desc->boardinfo->static_addr_method;
> +	i3c_bus_normaluse_unlock(i3cdev->bus);
>  
>  	/*
>  	 * SETAASA-based devices need not always have a matching ID since

[Severity: High]
This is a pre-existing issue, but I noticed a similar locking problem in
the AMD SBRMI driver.

In drivers/misc/amd-sbi/rmi-i2c.c:sbrmi_i3c_probe():

    if (I3C_PID_INSTANCE_ID(i3cdev->desc->info.pid) != 1)

This dereferences i3cdev->desc without acquiring i3c_bus_normaluse_lock()
or using the safe i3c_device_get_info() API.

Since probe runs holding device_lock(), it is not mutually exclusive with
the bus maintenance write-lock used for descriptor replacement in
__i3c_master_add_i3c_dev_locked(). If a device's address is concurrently
re-assigned during DAA hot-join, could probe dereference a freed descriptor
when it resumes?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804133810.184905-1-adrian.hunter@intel.com?part=7

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 02/14] i3c: Fix unlocked dereference of dev->desc in i3c_device_get_supported_xfer_mode()

[Adrian Hunter]

On 04/08/2026 18:09, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> New issues:
> - [Critical] The patch transforms a predictable NULL pointer dereference into an exploitable Use-After-Free of `master->this` when an I3C device outlives its master controller's unregistration.

No. 'desc' is never NULL however it could be being changed concurrently
prior to being freed.  UAF of 'desc' is no different to UAF of master->this
(which is anyway being addressed by a different patch
"i3c: master: Fix use-after-free of master->this"),
so nothing "transforms" here.

> 
> Pre-existing issues:
> - [High] Uninitialized stack memory (`devinfo`) is used for driver matching when `i3cdev->desc` is NULL.
> - [High] A Time-of-Check to Time-of-Use (TOCTOU) race condition exists when accessing `i3cdev->desc->boardinfo`, leading to a Use-After-Free or NULL pointer dereference.
> --
> 
> --- Patch [2]: [PATCH V3 02/14] i3c: Fix unlocked dereference of dev->desc in i3c_device_get_supported_xfer_mode() ---
> commit 97cfccca08da732124561cbc1aa49b571a84dd8c
> Author: Adrian Hunter <adrian.hunter@intel.com>
> 
> i3c: Fix unlocked dereference of dev->desc in i3c_device_get_supported_xfer_mode()
> 
> This patch modifies i3c_device_get_supported_xfer_mode() to use dev->bus
> instead of dev->desc to obtain the master controller. This avoids a potential
> unlocked dereference of dev->desc.
> 
>> diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
>> index 101eaa77de686..a3778282e84c2 100644
>> --- a/drivers/i3c/device.c
>> +++ b/drivers/i3c/device.c
>> @@ -309,7 +309,7 @@ EXPORT_SYMBOL_GPL(i3c_device_match_id);
>>   */
>>  u32 i3c_device_get_supported_xfer_mode(struct i3c_device *dev)
>>  {
>> -	return i3c_dev_get_master(dev->desc)->this->info.hdr_cap | BIT(I3C_SDR);
>> +	return i3c_bus_to_i3c_master(dev->bus)->this->info.hdr_cap | BIT(I3C_SDR);
> 
> [Severity: Critical]
> Does this change introduce a use-after-free if the device outlives the master
> controller?
> 
> When a master is unregistered, i3c_master_unregister() calls
> i3c_master_unregister_i3c_devs() to unregister child devices. If a child
> device has an elevated refcount (e.g., from an open file descriptor), its
> struct i3c_device outlives the unregistration.
> 
> i3c_master_bus_cleanup() then calls i3c_master_detach_free_devs(), which
> explicitly frees master->this.
> 
> If the child device later calls i3c_device_get_supported_xfer_mode(), it
> would dereference the explicitly freed master->this pointer. The previous
> code safely faulted on a NULL dev->desc, but this new path appears to access
> freed memory.
> 
> [ ... ]
> 
> [Severity: High]
> This is a pre-existing issue, but does i3c_device_match() have a TOCTOU race
> condition leading to a use-after-free?
> 
> drivers/i3c/master.c:i3c_device_match() {
>         if (i3cdev->desc && i3cdev->desc->boardinfo)
>                 static_addr_method = i3cdev->desc->boardinfo->static_addr_method;
> }
> 
> If i3c_device_match() accesses i3cdev->desc without holding the device lock
> during concurrent master unregistration, a context switch could occur right
> after i3cdev->desc is checked.
> 
> If the unregistration thread then frees the descriptor via kfree() in
> i3c_master_bus_cleanup(), the subsequent evaluation of
> i3cdev->desc->boardinfo will dereference a freed pointer.
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but can i3c_device_match_id()
> evaluate uninitialized stack memory?
> 
> During concurrent driver registration and master unregistration, if
> i3c_master_unregister_i3c_devs() sets i3cdev->dev->desc = NULL,
> i3c_device_match() might still proceed to call i3c_device_match_id().
> 
> drivers/i3c/device.c:i3c_device_match_id() {
>         struct i3c_device_info devinfo;
>         ...
>         i3c_device_get_info(i3cdev, &devinfo);
>         
>         manuf = I3C_PID_MANUF_ID(devinfo.pid);
> }
> 
> If i3cdev->desc is NULL, i3c_device_get_info() bails out safely without
> populating devinfo, leaving the stack memory completely uninitialized when
> it is subsequently read for driver matching.
> 


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 04/14] i3c: master: Fix use-after-free of master->this

[Adrian Hunter]

On 04/08/2026 17:10, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [High] A double free occurs in the error path of `i3c_master_set_info()` because `master->this` is left as a dangling pointer and is subsequently freed again in `i3c_masterdev_release()`.

That is valid.  i3c_master_set_info() needs to be amended either to set
master->this = NULL on the error path, or just don't call
i3c_master_free_i3c_dev() since it will get cleaned up in
i3c_masterdev_release() anyway.

> 
> Pre-existing issues:
> - [High] Devres lifetime mismatch allows use-after-free of the master controller struct in `i3c_masterdev_release()`.
> --
> 
> --- Patch [4]: [PATCH V3 04/14] i3c: master: Fix use-after-free of master->this ---
> commit e74d33e25ba87fdc800277aa5702a984d20076ca
> Author: Adrian Hunter <adrian.hunter@intel.com>
> i3c: master: Fix use-after-free of master->this
> 
> This patch delays freeing master->this until i3c_masterdev_release(). By
> doing this, it keeps master->this alive until all users of the master
> device have gone away and the sysfs state is torn down.
> 
>> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
>> index abb582645a2ed..4839c1c186eb0 100644
>> --- a/drivers/i3c/master.c
>> +++ b/drivers/i3c/master.c
> [ ... ]
>> @@ -842,6 +842,11 @@ static struct attribute *i3c_masterdev_attrs[] = {
>>  };
>>  ATTRIBUTE_GROUPS(i3c_masterdev);
>>  
>> +static void i3c_master_free_i3c_dev(struct i3c_dev_desc *dev)
>> +{
>> +	kfree(dev);
>> +}
>> +
>>  static void i3c_masterdev_release(struct device *dev)
>>  {
>>  	struct i3c_master_controller *master = dev_to_i3cmaster(dev);
>>  	struct i3c_bus *bus = dev_to_i3cbus(dev);
> 
> [Severity: High]
> This is a pre-existing issue, but does this release function dereference
> fields in the master struct after the driver's devres cleanup has already
> freed the surrounding memory?
> 
> Master controller structures are typically allocated using devm_kzalloc().
> When a driver is unbound, devres cleanup frees the master controller
> structure. However, if userspace holds an open file descriptor to a sysfs
> attribute (like /sys/class/i3c/i3c-0/bcr), the kobject refcount doesn't drop
> to zero.
> 
> When the sysfs file is later closed, the final put_device() calls this
> i3c_masterdev_release() function, which then dereferences fields like
> master->wq, bus->devs, and now master->this, leading to a use-after-free.
> 
>>  	if (master->wq)
>>  		destroy_workqueue(master->wq);
>>  
>>  	WARN_ON(!list_empty(&bus->devs.i2c) || !list_empty(&bus->devs.i3c));
>>  	i3c_bus_cleanup(bus);
>>  
>>  	fwnode_handle_put(dev->fwnode);
>> +
>> +	i3c_master_free_i3c_dev(master->this);
> 
> [Severity: High]
> Does unconditionally freeing master->this here cause a double free on the
> error path of i3c_master_set_info()?
> 
> If i3c_master_attach_i3c_dev() fails during initialization in
> i3c_master_set_info(), the allocated i3cdev is freed via
> i3c_master_free_i3c_dev().
> 
> However, master->this and master->bus.cur_master were already assigned to
> point to i3cdev and are not set back to NULL:
> 
> drivers/i3c/master.c:i3c_master_set_info() {
> 	...
> 	master->this = i3cdev;
> 	master->bus.cur_master = master->this;
> 
> 	ret = i3c_master_attach_i3c_dev(master, i3cdev);
> 	if (ret)
> 		goto err_free_dev;
> 	...
> err_free_dev:
> 	i3c_master_free_i3c_dev(i3cdev);
> 	return ret;
> }
> 
> The failure propagates to i3c_master_register(), which calls
> put_device(&master->dev) to abort driver registration. This calls
> i3c_masterdev_release(), which now unconditionally frees master->this,
> leading to a double free of the dangling pointer.
> 
>>  }
> 


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 09/14] i3c: master: Report wakeup events for IBIs

[Adrian Hunter]

On 04/08/2026 18:10, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [High] Unprotected dereference of dev->dev in i3c_master_queue_ibi() leading to NULL pointer dereference or Use-After-Free during teardown.

dev->dev is always valid.  It is use of the first 'dev' (struct i3c_dev_desc *)
that is questionable because of the oldev/newdev dance in
__i3c_master_add_i3c_dev_locked().  As it says there, olddev does not
exist on the bus, so it is unlikely there are any IBIs inflight from
when it did exist.  It would be strange if a newly DAA'ed device
started issuing IBIs, but most controller drivers do not accept IBIs
from newly DAA'ed devices anyway, so there ought not to be any IBIs
inflight for newdev either.

It is fragile, but it is not a new issue because struct i3c_dev_desc *dev
was/is being used anyway.

> 
> Pre-existing issues:
> - [High] Race condition between hardware IBI interrupts and IBI teardown leading to Use-After-Free of dev->ibi.
> --
> 
> --- Patch [9]: [PATCH V3 09/14] i3c: master: Report wakeup events for IBIs ---
> commit 044b30e8813c466ee0c955f282a081170370a564
> Author: Adrian Hunter <adrian.hunter@intel.com>
> 
> i3c: master: Report wakeup events for IBIs
> 
> When an IBI is queued for processing, this commit records a wakeup event
> for the device if wakeup is enabled. It uses a 100 ms processing interval
> to give the I3C device driver time to process the IBI.
> 
>> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
>> index e6b320da475e0..8c9e62e6f1469 100644
>> --- a/drivers/i3c/master.c
>> +++ b/drivers/i3c/master.c
>> @@ -3398,6 +3401,9 @@ void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot)
>>  	if (!dev->ibi || !slot)
>>  		return;
>>  
>> +	if (device_may_wakeup(&dev->dev->dev))
>> +		pm_wakeup_event(&dev->dev->dev, I3C_WAKEUP_PROCESSING_TIME_MS);
>> +
> 
> [Severity: High]
> Is it possible for an IBI interrupt to fire concurrently with device
> unregistration?
> 
> Looking at i3c_master_unregister_i3c_devs(), the teardown path sets
> dev->dev to NULL:
> 
> drivers/i3c/master.c:i3c_master_unregister_i3c_devs() {
>     ...
>     i3cdev->dev->desc = NULL;
>     put_device(&i3cdev->dev->dev);
>     i3cdev->dev = NULL;
>     ...
> }
> 
> If an interrupt occurs during or just after this unregistration, could
> dev->dev be NULL when device_may_wakeup() dereferences it here?
> 
>>  	atomic_inc(&dev->ibi->pending_ibis);
>>  	queue_work(dev->ibi->wq, &slot->work);
>>  }
> 
> [Severity: High]
> This is a pre-existing issue, but is there a race condition between hardware
> IBI interrupts and IBI teardown leading to a use-after-free of dev->ibi?
> 
> In i3c_dev_disable_ibi_locked(), the code checks the pending_ibis counter:
> 
> drivers/i3c/master.c:i3c_dev_disable_ibi_locked() {
>     ...
>     if (atomic_read(&dev->ibi->pending_ibis))
>         wait_for_completion(&dev->ibi->all_ibis_handled);
>     ...
> }
> 
> If a hardirq handler is already executing but has not yet called
> i3c_master_queue_ibi() to increment pending_ibis, the teardown path might
> assume no IBIs are pending and proceed to free dev->ibi in
> i3c_dev_free_ibi_locked().
> 
> When the interrupted CPU resumes, wouldn't it execute this atomic_inc() and
> queue_work() on the freed dev->ibi pointer since there is no
> synchronize_irq() to drain active hardirqs?
> 


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 01/14] i3c: master: Fix recursive locking during device registration

[Frank Li]

On Tue, Aug 04, 2026 at 04:37:57PM +0300, Adrian Hunter wrote:
> i3c_master_register_new_i3c_devs() registers newly discovered devices
> while holding i3c_bus_normaluse_lock(), a down_read().  device_register()
> can immediately probe the device, and probe callbacks typically invoke
> I3C helpers that take i3c_bus_normaluse_lock() again, leading to a
> recursive acquisition of the same rwsem.  rwsems do not support recursive
> read locking and can deadlock when a writer is waiting.  See the
> "Recursive read locks" section of Documentation/locking/lockdep-design.rst.
>
> For example, with Intel LPSS I3C, LOCKDEP generates a WARNING like:
>   # echo intel-lpss-i3c.0 > /sys/bus/platform/drivers/mipi-i3c-hci/unbind
>   # echo intel-lpss-i3c.0 > /sys/bus/platform/drivers/mipi-i3c-hci/bind
>   WARNING: possible recursive locking detected
>   kworker/5:1/94 is trying to acquire lock:
>   ffff88811c810d78 (&i3cbus->lock){++++}-{4:4}, at: i3c_device_match_id+0x45/0x370
>   but task is already holding lock:
>   ffff88811c810d78 (&i3cbus->lock){++++}-{4:4}, at: i3c_master_reg_work_fn+0x21/0x5f0
>
> Fix this by separating device creation from device registration.
> Populate desc->dev under the maintenance lock, collect the devices that
> still need registration into a local list, then release the lock before
> calling device_register().  Finally retake the lock and clean up any
> devices that failed to register.
>
> Use the maintenance lock rather than the normal-use lock while adding
> device objects.  A write-side maintenance lock prevents readers from
> observing a partially initialized desc->dev during initial device
> population, or desc->dev disappearing if registration fails.
>
> The local list requires a list node, so add a list node member to struct
> i3c_device.
>
> Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
> Cc: stable@vger.kernel.org
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>
>
> Changes in V3:
>
> 	Added Cc: stable@vger.kernel.org
>
> Changes in V2:
>
> 	New patch
>
>
>  drivers/i3c/master.c       | 45 ++++++++++++++++++++++++++++----------
>  include/linux/i3c/master.h |  2 ++
>  2 files changed, 35 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index f485b98805cf..d2fb1a110521 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -2069,12 +2069,21 @@ static int i3c_master_early_i3c_dev_add(struct i3c_master_controller *master,
>  static void
>  i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
>  {
> +	struct i3c_device *i3cdev, *tmp;
>  	struct i3c_dev_desc *desc;
> +	LIST_HEAD(i3c_unreg_devs);
>  	int ret;
>
>  	if (!master->init_done)
>  		return;
>
> +	i3c_bus_maintenance_lock(&master->bus);
> +
> +	if (master->shutting_down) {
> +		i3c_bus_maintenance_unlock(&master->bus);
> +		return;
> +	}
> +
>  	i3c_bus_for_each_i3cdev(&master->bus, desc) {
>  		if (desc->dev || !desc->info.dyn_addr || desc == master->this)
>  			continue;
> @@ -2104,25 +2113,37 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
>  		if (desc->boardinfo)
>  			device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);
>
> -		ret = device_register(&desc->dev->dev);
> -		if (ret) {
> -			dev_err(&master->dev,
> -				"Failed to add I3C device (err = %d)\n", ret);
> -			desc->dev->desc = NULL;
> -			put_device(&desc->dev->dev);
> -			desc->dev = NULL;
> -		}
> +		list_add_tail(&desc->dev->node, &i3c_unreg_devs);
> +	}
> +
> +	i3c_bus_maintenance_unlock(&master->bus);
> +
> +	list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
> +		ret = device_register(&i3cdev->dev);
> +		if (ret)
> +			dev_err(&master->dev, "Failed to add I3C device (err = %d)\n", ret);
> +		else
> +			list_del_init(&i3cdev->node);

Is it risk del node without acquire lock?

Frank

> +	}
> +
> +	i3c_bus_maintenance_lock(&master->bus);
> +
> +	list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
> +		list_del(&i3cdev->node);
> +		desc = i3cdev->desc;
> +		i3cdev->desc = NULL;
> +		put_device(&i3cdev->dev);
> +		desc->dev = NULL;
>  	}
> +
> +	i3c_bus_maintenance_unlock(&master->bus);
>  }
>
>  static void i3c_master_reg_work_fn(struct work_struct *work)
>  {
>  	struct i3c_master_controller *master = container_of(work, typeof(*master), reg_work);
>
> -	i3c_bus_normaluse_lock(&master->bus);
> -	if (!master->shutting_down)
> -		i3c_master_register_new_i3c_devs(master);
> -	i3c_bus_normaluse_unlock(&master->bus);
> +	i3c_master_register_new_i3c_devs(master);
>  }
>
>  /**
> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
> index 2dc139a217bf..2b96c4ea75fb 100644
> --- a/include/linux/i3c/master.h
> +++ b/include/linux/i3c/master.h
> @@ -238,6 +238,7 @@ struct i3c_dev_desc {
>   *	  every time the I3C device is rediscovered with a different dynamic
>   *	  address assigned
>   * @bus: I3C bus this device is attached to
> + * @node: unregistered device list node
>   *
>   * I3C device object exposed to I3C device drivers. The takes care of linking
>   * this object to the relevant &struct_i3c_dev_desc one.
> @@ -248,6 +249,7 @@ struct i3c_device {
>  	struct device dev;
>  	struct i3c_dev_desc *desc;
>  	struct i3c_bus *bus;
> +	struct list_head node;
>  };
>
>  /*
> --
> 2.53.0
>

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 03/14] i3c: master: Do not treat master device as a duplicate target

[Frank Li]

On Tue, Aug 04, 2026 at 04:37:59PM +0300, Adrian Hunter wrote:
> i3c_master_search_i3c_dev_duplicate() searches the bus for another I3C
> device with the same PID as the reference device.  The search can match
> master->this, causing the controller itself to be returned as a
> duplicate.
>
> Since the controller is not a target device, it cannot be a duplicate of
> one.  Exclude master->this from matching so that the function only
> returns real duplicate target devices.
>
> Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
> Cc: stable@vger.kernel.org
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
>
> Changes in V3:
>
> 	New patch
>
>
>  drivers/i3c/master.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index c7bb52b71d88..abb582645a2e 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -2545,7 +2545,8 @@ i3c_master_search_i3c_dev_duplicate(struct i3c_dev_desc *refdev)
>
>  	i3c_bus_for_each_i3cdev(&master->bus, i3cdev) {
>  		if (i3cdev != refdev && i3cdev->info.pid &&
> -		    i3cdev->info.pid == refdev->info.pid)
> +		    i3cdev->info.pid == refdev->info.pid &&
> +		    i3cdev != master->this)
>  			return i3cdev;
>  	}
>
> --
> 2.53.0
>

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 01/14] i3c: master: Fix recursive locking during device registration

[Adrian Hunter]

On 04/08/2026 19:46, Frank Li wrote:
> On Tue, Aug 04, 2026 at 04:37:57PM +0300, Adrian Hunter wrote:
>> i3c_master_register_new_i3c_devs() registers newly discovered devices
>> while holding i3c_bus_normaluse_lock(), a down_read().  device_register()
>> can immediately probe the device, and probe callbacks typically invoke
>> I3C helpers that take i3c_bus_normaluse_lock() again, leading to a
>> recursive acquisition of the same rwsem.  rwsems do not support recursive
>> read locking and can deadlock when a writer is waiting.  See the
>> "Recursive read locks" section of Documentation/locking/lockdep-design.rst.
>>
>> For example, with Intel LPSS I3C, LOCKDEP generates a WARNING like:
>>   # echo intel-lpss-i3c.0 > /sys/bus/platform/drivers/mipi-i3c-hci/unbind
>>   # echo intel-lpss-i3c.0 > /sys/bus/platform/drivers/mipi-i3c-hci/bind
>>   WARNING: possible recursive locking detected
>>   kworker/5:1/94 is trying to acquire lock:
>>   ffff88811c810d78 (&i3cbus->lock){++++}-{4:4}, at: i3c_device_match_id+0x45/0x370
>>   but task is already holding lock:
>>   ffff88811c810d78 (&i3cbus->lock){++++}-{4:4}, at: i3c_master_reg_work_fn+0x21/0x5f0
>>
>> Fix this by separating device creation from device registration.
>> Populate desc->dev under the maintenance lock, collect the devices that
>> still need registration into a local list, then release the lock before
>> calling device_register().  Finally retake the lock and clean up any
>> devices that failed to register.
>>
>> Use the maintenance lock rather than the normal-use lock while adding
>> device objects.  A write-side maintenance lock prevents readers from
>> observing a partially initialized desc->dev during initial device
>> population, or desc->dev disappearing if registration fails.
>>
>> The local list requires a list node, so add a list node member to struct
>> i3c_device.
>>
>> Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>
>>
>> Changes in V3:
>>
>> 	Added Cc: stable@vger.kernel.org
>>
>> Changes in V2:
>>
>> 	New patch
>>
>>
>>  drivers/i3c/master.c       | 45 ++++++++++++++++++++++++++++----------
>>  include/linux/i3c/master.h |  2 ++
>>  2 files changed, 35 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
>> index f485b98805cf..d2fb1a110521 100644
>> --- a/drivers/i3c/master.c
>> +++ b/drivers/i3c/master.c
>> @@ -2069,12 +2069,21 @@ static int i3c_master_early_i3c_dev_add(struct i3c_master_controller *master,
>>  static void
>>  i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
>>  {
>> +	struct i3c_device *i3cdev, *tmp;
>>  	struct i3c_dev_desc *desc;
>> +	LIST_HEAD(i3c_unreg_devs);
>>  	int ret;
>>
>>  	if (!master->init_done)
>>  		return;
>>
>> +	i3c_bus_maintenance_lock(&master->bus);
>> +
>> +	if (master->shutting_down) {
>> +		i3c_bus_maintenance_unlock(&master->bus);
>> +		return;
>> +	}
>> +
>>  	i3c_bus_for_each_i3cdev(&master->bus, desc) {
>>  		if (desc->dev || !desc->info.dyn_addr || desc == master->this)
>>  			continue;
>> @@ -2104,25 +2113,37 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
>>  		if (desc->boardinfo)
>>  			device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);
>>
>> -		ret = device_register(&desc->dev->dev);
>> -		if (ret) {
>> -			dev_err(&master->dev,
>> -				"Failed to add I3C device (err = %d)\n", ret);
>> -			desc->dev->desc = NULL;
>> -			put_device(&desc->dev->dev);
>> -			desc->dev = NULL;
>> -		}
>> +		list_add_tail(&desc->dev->node, &i3c_unreg_devs);
>> +	}
>> +
>> +	i3c_bus_maintenance_unlock(&master->bus);
>> +
>> +	list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
>> +		ret = device_register(&i3cdev->dev);
>> +		if (ret)
>> +			dev_err(&master->dev, "Failed to add I3C device (err = %d)\n", ret);
>> +		else
>> +			list_del_init(&i3cdev->node);
> 
> Is it risk del node without acquire lock?

The list head is local i3c_unreg_devs, and i3c_master_register_new_i3c_devs()
is not permitted to race with itself, by being called only from the work
function i3c_master_reg_work_fn()

> 
> Frank
> 
>> +	}
>> +
>> +	i3c_bus_maintenance_lock(&master->bus);
>> +
>> +	list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
>> +		list_del(&i3cdev->node);
>> +		desc = i3cdev->desc;
>> +		i3cdev->desc = NULL;
>> +		put_device(&i3cdev->dev);
>> +		desc->dev = NULL;
>>  	}
>> +
>> +	i3c_bus_maintenance_unlock(&master->bus);
>>  }
>>
>>  static void i3c_master_reg_work_fn(struct work_struct *work)
>>  {
>>  	struct i3c_master_controller *master = container_of(work, typeof(*master), reg_work);
>>
>> -	i3c_bus_normaluse_lock(&master->bus);
>> -	if (!master->shutting_down)
>> -		i3c_master_register_new_i3c_devs(master);
>> -	i3c_bus_normaluse_unlock(&master->bus);
>> +	i3c_master_register_new_i3c_devs(master);
>>  }
>>
>>  /**
>> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
>> index 2dc139a217bf..2b96c4ea75fb 100644
>> --- a/include/linux/i3c/master.h
>> +++ b/include/linux/i3c/master.h
>> @@ -238,6 +238,7 @@ struct i3c_dev_desc {
>>   *	  every time the I3C device is rediscovered with a different dynamic
>>   *	  address assigned
>>   * @bus: I3C bus this device is attached to
>> + * @node: unregistered device list node
>>   *
>>   * I3C device object exposed to I3C device drivers. The takes care of linking
>>   * this object to the relevant &struct_i3c_dev_desc one.
>> @@ -248,6 +249,7 @@ struct i3c_device {
>>  	struct device dev;
>>  	struct i3c_dev_desc *desc;
>>  	struct i3c_bus *bus;
>> +	struct list_head node;
>>  };
>>
>>  /*
>> --
>> 2.53.0
>>


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 07/14] i3c: master: Fix potential UAF in i3c_device_match()

[Adrian Hunter]

On 04/08/2026 18:11, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> New issues:
> - [High] Removal of the NULL check for `i3cdev->desc` introduces a guaranteed NULL pointer dereference in concurrent unregistration paths.

Not valid

> - [High] Unconditional access to `i3cdev->bus` in `i3c_device_match()` causes a Use-After-Free if the parent master controller has been freed.

Makes no sense.

> 
> Pre-existing issues:
> - [High] Preexisting use-after-free race condition in AMD SBRMI (`sbrmi_i3c_probe`) accessing `i3cdev->desc` without holding the bus lock.
> --
> 
> --- Patch [7]: [PATCH V3 07/14] i3c: master: Fix potential UAF in i3c_device_match() ---
> commit d088359c0cccb56067d426e92d0a49f24376de53
> Author: Adrian Hunter <adrian.hunter@intel.com>
> 
> i3c: master: Fix potential UAF in i3c_device_match()
> 
> This commit protects access to i3cdev->desc with the bus normal-use lock
> in i3c_device_match() to prevent a use-after-free during concurrent
> descriptor replacement. It also removes the NULL check for i3cdev->desc,
> assuming the lock guarantees validity.
> 
>> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
>> index 947ab3c681d5e..e7ea410078891 100644
>> --- a/drivers/i3c/master.c
>> +++ b/drivers/i3c/master.c
>> @@ -347,8 +347,10 @@ static int i3c_device_match(struct device *dev, const struct device_driver *drv)
>>  	i3cdev = dev_to_i3cdev(dev);
>>  	i3cdrv = drv_to_i3cdrv(drv);
>>  
>> -	if (i3cdev->desc && i3cdev->desc->boardinfo)
>> +	i3c_bus_normaluse_lock(i3cdev->bus);
> 
> [Severity: High]
> Does accessing i3cdev->bus here risk a use-after-free if the parent master
> controller has been freed?
> 
> During unregistration, i3c_master_unregister() unregisters child devices,
> and device_del() drops the child's reference to the parent by calling
> put_device(parent). Once unregistration finishes, the parent platform device
> unbinds and its devres frees the I3C master struct. 
> 
> If a concurrent bus_for_each_dev() iterator (such as from driver registration)

That makes no sense.

> holds a reference to the child device, the child outlives the parent. The
> child's i3cdev->bus pointer would then remain pointing to the freed master
> memory, leading to a use-after-free when acquiring the lock.
> 
>> +	if (i3cdev->desc->boardinfo)
> 
> [Severity: High]
> Could removing the NULL check for i3cdev->desc lead to a NULL pointer
> dereference during concurrent unregistration?
> 
> In i3c_master_unregister_i3c_devs(), i3cdev->dev->desc = NULL; is set without
> holding any bus locks. 

The DAA and registration paths are already blocked by
master->shutting_down flag - refer i3c_master_shutdown()

> 
> If a concurrent driver matching thread invokes i3c_device_match(), it can
> proceed in parallel because driver_match_device() operates outside
> device_lock(). The thread acquires the bus lock, but since unregistration
> doesn't hold it, it could read desc as NULL and crash when accessing
> i3cdev->desc->boardinfo.
> 
>>  		static_addr_method = i3cdev->desc->boardinfo->static_addr_method;
>> +	i3c_bus_normaluse_unlock(i3cdev->bus);
>>  
>>  	/*
>>  	 * SETAASA-based devices need not always have a matching ID since
> 
> [Severity: High]
> This is a pre-existing issue, but I noticed a similar locking problem in
> the AMD SBRMI driver.
> 
> In drivers/misc/amd-sbi/rmi-i2c.c:sbrmi_i3c_probe():
> 
>     if (I3C_PID_INSTANCE_ID(i3cdev->desc->info.pid) != 1)
> 
> This dereferences i3cdev->desc without acquiring i3c_bus_normaluse_lock()
> or using the safe i3c_device_get_info() API.
> 
> Since probe runs holding device_lock(), it is not mutually exclusive with
> the bus maintenance write-lock used for descriptor replacement in
> __i3c_master_add_i3c_dev_locked(). If a device's address is concurrently
> re-assigned during DAA hot-join, could probe dereference a freed descriptor
> when it resumes?
> 


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 03/14] i3c: master: Do not treat master device as a duplicate target

[Mukesh Savaliya]

On 8/4/2026 7:07 PM, Adrian Hunter wrote:
> i3c_master_search_i3c_dev_duplicate() searches the bus for another I3C
> device with the same PID as the reference device.  The search can match
> master->this, causing the controller itself to be returned as a
> duplicate.
> 
> Since the controller is not a target device, it cannot be a duplicate of
> one.  Exclude master->this from matching so that the function only
> returns real duplicate target devices.
> 
> Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
> Cc: stable@vger.kernel.org
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> 
> 
> Changes in V3:
> 
> 	New patch
> 
> 
>   drivers/i3c/master.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 

Acked-by: Mukesh Savaliya <mukesh.savaliya@oss.qualcomm.com>

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 05/14] i3c: Make dev->desc locking assumptions explicit

[Mukesh Savaliya]

On 8/4/2026 7:08 PM, Adrian Hunter wrote:
> i3c_device_get_info() takes the bus normal-use lock before accessing
> dev->desc. Under that lock, the descriptor pointer is guaranteed to be
> valid for the duration of the access.
> 
> Remove the unnecessary NULL check on dev->desc so the code more clearly
> reflects the locking rules and expected descriptor lifetime.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> 
> 
> Changes in V3:
> 
> 	New patch
> 
> 
>   drivers/i3c/device.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
> index a3778282e84c..5e6df6de0283 100644

Acked-by: Mukesh Savaliya <mukesh.savaliya@oss.qualcomm.com>


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 06/14] i3c: master: Fix potential UAF in i3c_device_uevent()

[Mukesh Savaliya]

On 8/4/2026 7:08 PM, Adrian Hunter wrote:
> i3c_device_uevent() dereferences i3cdev->desc without holding the bus
> normal-use lock.  Since the descriptor pointer can be replaced
> concurrently, including when a uevent is generated from sysfs, this can
> result in dereferencing a stale descriptor and lead to a use-after-free.
> 
> Use i3c_device_get_info() instead, which protects access to the
> descriptor with the normal-use lock.
> 
> Commit 6cf7b65f7029 ("i3c: Use i3cdev->desc->info instead of calling
> i3c_device_get_info() to avoid deadlock") replaced the accessor with a
> direct descriptor dereference because i3c_device_get_info() would
> recursively acquire bus->lock during device registration.
> 
> This change depends on "i3c: master: Fix recursive locking during device
> registration", which moves device registration out from under bus->lock
> and removes the possibility of that deadlock.  Without that change,
> restoring the i3c_device_get_info() call would reintroduce the deadlock.
> 
> Fixes: 6cf7b65f7029 ("i3c: Use i3cdev->desc->info instead of calling i3c_device_get_info() to avoid deadlock")
> Cc: stable@vger.kernel.org # requires "i3c: master: Fix recursive locking during device registration"
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> 
> 
> Changes in V3:
> 
> 	New patch
> 
> 
>   drivers/i3c/master.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c

Acked-by: Mukesh Savaliya <mukesh.savaliya@oss.qualcomm.com>

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 08/14] i3c: master: Support IBI-based wakeup capability

[Mukesh Savaliya]

On 8/4/2026 7:08 PM, Adrian Hunter wrote:
[...]
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index e7ea41007889..e6b320da475e 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -2110,6 +2110,13 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
>   		if (desc->boardinfo)
>   			device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);
>   
> +		/*
> +		 * In the case of IBI wakeup, any IBI-capable device can
> +		 * wakeup.
you are registering a particular device to the master, so why "any 
IBI-capable device " ?

"If device has IBI capability, mark as wakeup capable" to simplify 
comment  ?
> +		 */
> +		if (master->ibi_wakeup && (desc->info.bcr & I3C_BCR_IBI_REQ_CAP))
> +			device_set_wakeup_capable(&desc->dev->dev, true);
> +
>   		list_add_tail(&desc->dev->node, &i3c_unreg_devs);
>   	}
>   
> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
> index 2b96c4ea75fb..16394aca7230 100644
> --- a/include/linux/i3c/master.h
> +++ b/include/linux/i3c/master.h
> @@ -523,6 +523,7 @@ struct i3c_master_controller_ops {
>    * @hotjoin: true if the master support hotjoin
>    * @rpm_allowed: true if Runtime PM allowed
>    * @rpm_ibi_allowed: true if IBI and Hot-Join allowed while runtime suspended
> + * @ibi_wakeup: IBI can wakeup the system
>    * @shutting_down: set to true when master begins shutdown or unregister
>    * @boardinfo.i3c: list of I3C  boardinfo objects
>    * @boardinfo.i2c: list of I2C boardinfo objects
> @@ -562,6 +563,7 @@ struct i3c_master_controller {
>   	unsigned int hotjoin: 1;
>   	unsigned int rpm_allowed: 1;
>   	unsigned int rpm_ibi_allowed: 1;
> +	unsigned int ibi_wakeup: 1;
>   	bool shutting_down;
>   	struct {
>   		struct list_head i3c;


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 10/14] i3c: master: Add helper to query bus wakeup requirements

[Mukesh Savaliya]

On 8/4/2026 7:08 PM, Adrian Hunter wrote:
[...]

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 8c9e62e6f146..baf4769f0512 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -2150,6 +2150,41 @@ static void i3c_master_reg_work_fn(struct work_struct *work)
>   	i3c_master_register_new_i3c_devs(master);
>   }
>   
> +/**
> + * i3c_master_any_wakeup_enabled() - check if any device can wake the system
> + * @master: I3C master controller
> + *
> + * Iterate over devices on the bus and return true if any device has
> + * system wakeup enabled and IBI enabled.
> + *
> + * Whether a device is enabled for system wakeup is user space policy,
> + * settable at any time through the device's power/wakeup sysfs attribute,
> + * so the answer is only stable once user space is frozen.  Call this from
> + * a system suspend callback.
> + *
> + * Return: true if any device may wake the system via IBI, false otherwise.
> + */
> +bool i3c_master_any_wakeup_enabled(struct i3c_master_controller *master)

wanted to suggest if this function name suits 
?i3c_master_has_wakeup_enabled_devs()
> +{
> +	struct i3c_dev_desc *desc;
> +	bool wakeup = false;
> +
> +	i3c_bus_normaluse_lock(&master->bus);
> +	i3c_bus_for_each_i3cdev(&master->bus, desc) {
> +		if (!desc->dev || desc == master->this || !device_may_wakeup(&desc->dev->dev))
> +			continue;
> +		guard(mutex)(&desc->ibi_lock);
> +		if (desc->ibi && desc->ibi->enabled) {
> +			wakeup = true;
> +			break;
> +		}
> +	}
> +	i3c_bus_normaluse_unlock(&master->bus);
> +
> +	return wakeup;
> +}
> +EXPORT_SYMBOL_GPL(i3c_master_any_wakeup_enabled);\[...]
[...]

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 13/14] i3c: mipi-i3c-hci: Factor out i3c_hci_sysdev()

[Mukesh Savaliya]

Hi Adrian,

On 8/4/2026 7:08 PM, Adrian Hunter wrote:

[...]

> @@ -117,6 +118,17 @@ static inline struct i3c_hci *to_i3c_hci(struct i3c_master_controller *m)
>   	return container_of(m, struct i3c_hci, master);
>   }
>   
> +/*
> + * Determine the device that does PM / DMA and has IOMMU setup done for it in
> + * case of enabled IOMMU (for use with the DMA API).
> + * Such device is either "mipi-i3c-hci" platform device (OF/ACPI enumeration)
> + * parent or grandparent (PCI enumeration).
> + */
I was kind of confused and could not understand much (excuse me!). But 
could draft below from function definition. I know comments removed from 
below function and kept here.

Hope this can be simplified or improved further if possible.

/**
* i3c_hci_sysdev() - Get the device used for system PM and DMA
* operations
* @dev: HCI device
*
* Return the PCI parent device when the HCI controller is attached
* through PCI, otherwise return @dev itself. The returned device can
* be used for system power management and wakeup configuration.
*
* Return: Device to use for system PM and wakeup handling.
*/
> +struct device *i3c_hci_sysdev(struct device *dev)
> +{
> +	return dev->parent && dev_is_pci(dev->parent) ? dev->parent : dev;
> +}
> +
>   static void i3c_hci_set_master_dyn_addr(struct i3c_hci *hci)
>   {
>   	reg_write(MASTER_DEVICE_ADDR,
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> index 0672ed1132f8..7c2b20474130 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> @@ -15,7 +15,6 @@
>   #include <linux/errno.h>
>   #include <linux/i3c/master.h>
>   #include <linux/io.h>
> -#include <linux/pci.h>
>   
>   #include "hci.h"
>   #include "cmd.h"
> @@ -301,23 +300,11 @@ static int hci_dma_init(struct i3c_hci *hci)
>   {
>   	struct hci_rings_data *rings;
>   	struct hci_rh_data *rh;
> -	struct device *sysdev;
>   	u32 regval;
>   	unsigned int i, nr_rings, xfers_sz, resps_sz;
>   	unsigned int ibi_status_ring_sz, ibi_data_ring_sz;
>   	int ret;
>   
> -	/*
> -	 * Set pointer to a physical device that does DMA and has IOMMU setup
> -	 * done for it in case of enabled IOMMU and use it with the DMA API.
> -	 * Here such device is either
> -	 * "mipi-i3c-hci" platform device (OF/ACPI enumeration) parent or
> -	 * grandparent (PCI enumeration).
> -	 */
> -	sysdev = hci->master.dev.parent;
> -	if (sysdev->parent && dev_is_pci(sysdev->parent))
> -		sysdev = sysdev->parent;
> -
>   	regval = rhs_reg_read(CONTROL);
>   	nr_rings = FIELD_GET(MAX_HEADER_COUNT_CAP, regval);
>   	dev_dbg(&hci->master.dev, "%d DMA rings available\n", nr_rings);
> @@ -332,7 +319,7 @@ static int hci_dma_init(struct i3c_hci *hci)
>   		return -ENOMEM;
>   	hci->io_data = rings;
>   	rings->total = nr_rings;
> -	rings->sysdev = sysdev;
> +	rings->sysdev = i3c_hci_sysdev(hci->master.dev.parent);
>   
>   	for (i = 0; i < rings->total; i++) {
>   		u32 offset = rhs_reg_read(RHn_OFFSET(i));
> diff --git a/drivers/i3c/master/mipi-i3c-hci/hci.h b/drivers/i3c/master/mipi-i3c-hci/hci.h
> index b3d9803b1968..b8d2a3d680f8 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/hci.h
> +++ b/drivers/i3c/master/mipi-i3c-hci/hci.h
> @@ -184,6 +184,8 @@ void amd_set_resp_buf_thld(struct i3c_hci *hci);
>   void i3c_hci_sync_irq_inactive(struct i3c_hci *hci);
>   int i3c_hci_process_xfer(struct i3c_hci *hci, struct hci_xfer *xfer, int n);
>   
> +struct device *i3c_hci_sysdev(struct device *dev);
> +
>   #define DEFAULT_AUTOSUSPEND_DELAY_MS 1000
>   
>   int i3c_hci_rpm_suspend(struct device *dev);


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V3 01/14] i3c: master: Fix recursive locking during device registration

[Frank Li]

On Tue, Aug 04, 2026 at 07:50:35PM +0300, Adrian Hunter wrote:
> On 04/08/2026 19:46, Frank Li wrote:
> > On Tue, Aug 04, 2026 at 04:37:57PM +0300, Adrian Hunter wrote:
> >> i3c_master_register_new_i3c_devs() registers newly discovered devices
> >> while holding i3c_bus_normaluse_lock(), a down_read().  device_register()
> >> can immediately probe the device, and probe callbacks typically invoke
> >> I3C helpers that take i3c_bus_normaluse_lock() again, leading to a
> >> recursive acquisition of the same rwsem.  rwsems do not support recursive
> >> read locking and can deadlock when a writer is waiting.  See the
> >> "Recursive read locks" section of Documentation/locking/lockdep-design.rst.
> >>
> >> For example, with Intel LPSS I3C, LOCKDEP generates a WARNING like:
> >>   # echo intel-lpss-i3c.0 > /sys/bus/platform/drivers/mipi-i3c-hci/unbind
> >>   # echo intel-lpss-i3c.0 > /sys/bus/platform/drivers/mipi-i3c-hci/bind
> >>   WARNING: possible recursive locking detected
> >>   kworker/5:1/94 is trying to acquire lock:
> >>   ffff88811c810d78 (&i3cbus->lock){++++}-{4:4}, at: i3c_device_match_id+0x45/0x370
> >>   but task is already holding lock:
> >>   ffff88811c810d78 (&i3cbus->lock){++++}-{4:4}, at: i3c_master_reg_work_fn+0x21/0x5f0
> >>
> >> Fix this by separating device creation from device registration.
> >> Populate desc->dev under the maintenance lock, collect the devices that
> >> still need registration into a local list, then release the lock before
> >> calling device_register().  Finally retake the lock and clean up any
> >> devices that failed to register.
> >>
> >> Use the maintenance lock rather than the normal-use lock while adding
> >> device objects.  A write-side maintenance lock prevents readers from
> >> observing a partially initialized desc->dev during initial device
> >> population, or desc->dev disappearing if registration fails.
> >>
> >> The local list requires a list node, so add a list node member to struct
> >> i3c_device.
> >>
> >> Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> >> ---
> >>
> >>
> >> Changes in V3:
> >>
> >> 	Added Cc: stable@vger.kernel.org
> >>
> >> Changes in V2:
> >>
> >> 	New patch
> >>
> >>
> >>  drivers/i3c/master.c       | 45 ++++++++++++++++++++++++++++----------
> >>  include/linux/i3c/master.h |  2 ++
> >>  2 files changed, 35 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> >> index f485b98805cf..d2fb1a110521 100644
> >> --- a/drivers/i3c/master.c
> >> +++ b/drivers/i3c/master.c
> >> @@ -2069,12 +2069,21 @@ static int i3c_master_early_i3c_dev_add(struct i3c_master_controller *master,
> >>  static void
> >>  i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
> >>  {
> >> +	struct i3c_device *i3cdev, *tmp;
> >>  	struct i3c_dev_desc *desc;
> >> +	LIST_HEAD(i3c_unreg_devs);
> >>  	int ret;
> >>
> >>  	if (!master->init_done)
> >>  		return;
> >>
> >> +	i3c_bus_maintenance_lock(&master->bus);
> >> +
> >> +	if (master->shutting_down) {
> >> +		i3c_bus_maintenance_unlock(&master->bus);
> >> +		return;
> >> +	}
> >> +
> >>  	i3c_bus_for_each_i3cdev(&master->bus, desc) {
> >>  		if (desc->dev || !desc->info.dyn_addr || desc == master->this)
> >>  			continue;
> >> @@ -2104,25 +2113,37 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
> >>  		if (desc->boardinfo)
> >>  			device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);
> >>
> >> -		ret = device_register(&desc->dev->dev);
> >> -		if (ret) {
> >> -			dev_err(&master->dev,
> >> -				"Failed to add I3C device (err = %d)\n", ret);
> >> -			desc->dev->desc = NULL;
> >> -			put_device(&desc->dev->dev);
> >> -			desc->dev = NULL;
> >> -		}
> >> +		list_add_tail(&desc->dev->node, &i3c_unreg_devs);
> >> +	}
> >> +
> >> +	i3c_bus_maintenance_unlock(&master->bus);
> >> +
> >> +	list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
> >> +		ret = device_register(&i3cdev->dev);
> >> +		if (ret)
> >> +			dev_err(&master->dev, "Failed to add I3C device (err = %d)\n", ret);
> >> +		else
> >> +			list_del_init(&i3cdev->node);
> >
> > Is it risk del node without acquire lock?
>
> The list head is local i3c_unreg_devs, and i3c_master_register_new_i3c_devs()
> is not permitted to race with itself, by being called only from the work
> function i3c_master_reg_work_fn()

Add comments in struct list_head node; in case someone reuse it without
lock at other place

Frank

>
> >
> > Frank
> >
> >> +	}
> >> +
> >> +	i3c_bus_maintenance_lock(&master->bus);
> >> +
> >> +	list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
> >> +		list_del(&i3cdev->node);
> >> +		desc = i3cdev->desc;
> >> +		i3cdev->desc = NULL;
> >> +		put_device(&i3cdev->dev);
> >> +		desc->dev = NULL;
> >>  	}
> >> +
> >> +	i3c_bus_maintenance_unlock(&master->bus);
> >>  }
> >>
> >>  static void i3c_master_reg_work_fn(struct work_struct *work)
> >>  {
> >>  	struct i3c_master_controller *master = container_of(work, typeof(*master), reg_work);
> >>
> >> -	i3c_bus_normaluse_lock(&master->bus);
> >> -	if (!master->shutting_down)
> >> -		i3c_master_register_new_i3c_devs(master);
> >> -	i3c_bus_normaluse_unlock(&master->bus);
> >> +	i3c_master_register_new_i3c_devs(master);
> >>  }
> >>
> >>  /**
> >> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
> >> index 2dc139a217bf..2b96c4ea75fb 100644
> >> --- a/include/linux/i3c/master.h
> >> +++ b/include/linux/i3c/master.h
> >> @@ -238,6 +238,7 @@ struct i3c_dev_desc {
> >>   *	  every time the I3C device is rediscovered with a different dynamic
> >>   *	  address assigned
> >>   * @bus: I3C bus this device is attached to
> >> + * @node: unregistered device list node
> >>   *
> >>   * I3C device object exposed to I3C device drivers. The takes care of linking
> >>   * this object to the relevant &struct_i3c_dev_desc one.
> >> @@ -248,6 +249,7 @@ struct i3c_device {
> >>  	struct device dev;
> >>  	struct i3c_dev_desc *desc;
> >>  	struct i3c_bus *bus;
> >> +	struct list_head node;
> >>  };
> >>
> >>  /*
> >> --
> >> 2.53.0
> >>
>

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c