i3c: Support IBI-based system wakeup

PATCH Applied Adrian Hunter 4 participants 32 messages Aug 7 – Aug 8

Subsystems (2)

PCI I3C

lore

Share & tools

lore

Adrian Hunter

author

2026-08-07 14:57 UTC #0 lore raw

v5 cover i3c: Support IBI-based system wakeup

Note: all patches except patch 7 now have Frank's Rev'd-by

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 V5:

Added Frank Li's Reviewed-by and Mukesh Savaliya's Acked-by tags.

Changes in V4:

i3c: master: Fix recursive locking during device registration
Expanded the kernel-doc for the new struct i3c_device @node member to note that it is only for use by i3c_master_register_new_i3c_devs() and is not protected by a lock.

i3c: master: Fix use-after-free of master->this
Also reset master->this and bus.cur_master to NULL on the i3c_master_set_info() error path before freeing the allocated device. Tidied up the commit message wording.

i3c: master: Support IBI-based wakeup capability
Shortened the comment about IBI-capable devices being wakeup capable.

i3c: master: Add helper to query bus wakeup requirements
Renamed i3c_master_any_wakeup_enabled() to i3c_master_has_wakeup_enabled_devs().

i3c: mipi-i3c-hci-pci: Propagate I3C wakeup requirements to PCI
Updated for the rename of i3c_master_any_wakeup_enabled() to i3c_master_has_wakeup_enabled_devs().

i3c: mipi-i3c-hci: Factor out i3c_hci_sysdev()
Reworked the comment that moves with the code into kernel-doc, and mentioned that in the commit message.

i3c: mipi-i3c-hci: Advertise IBI wakeup capability
Updated the i3c_hci_sysdev() kernel-doc to mention system PM and wakeup, in place of the old comment tweak.

Added Frank Li's Reviewed-by and Mukesh Savaliya's Acked-by tags.

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: [email protected]

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                               | 119 ++++++++++++++++-----
 drivers/i3c/master/mipi-i3c-hci/core.c             |  20 ++++
 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                         |   6 ++
 8 files changed, 155 insertions(+), 48 deletions(-)

Regards
Adrian

Adrian Hunter

author

2026-08-07 14:57 UTC #1 lore raw

PATCH v5 1/14 i3c: master: Fix recursive locking during device registration

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 [email protected]

Signed-off-by Adrian Hunter <[email protected]>

Reviewed-by Frank Li <[email protected]>

---

Changes in V5:

Added Frank's Rev'd-by

Changes in V4:

Expanded the kernel-doc for the new struct i3c_device @node member to note that it is only for use by i3c_master_register_new_i3c_devs() and is not protected by a lock.

Changes in V3:

Added Cc: [email protected]

Changes in V2:

New patch

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

drivers/i3c/master.c +33 ~~-12~~

index f485b98805cf..d2fb1a110521 100644
@@ -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);
 }
 
 /**

include/linux/i3c/master.h +3 -0

index 2dc139a217bf..26535beb1e77 100644
@@ -238,6 +238,8 @@ 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, only for use by
+ *	  i3c_master_register_new_i3c_devs(), it is not protected by a lock
  *
  * 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 +250,7 @@ struct i3c_device {
 	struct device dev;
 	struct i3c_dev_desc *desc;
 	struct i3c_bus *bus;
+	struct list_head node;
 };
 
 /*

Signature · 1 line

2.53.0

[email protected]

AI review

2026-08-07 15:26 UTC #2 lore raw

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

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

New issues:
- [High] Dropping the bus lock during device_register() allows concurrent Dynamic Address Assignment (DAA) to free the underlying I3C device descriptor (i3cdev->desc), leading to a Use-After-Free in the uevent callback.

Pre-existing issues:
- [High] Unbalanced fwnode_handle_put in i3c_device_release leads to a refcount underflow of the boardinfo fwnode on device registration failure. --

--- Patch [1]: [PATCH V5 01/14] i3c: master: Fix recursive locking during device registration --- commit 7ad9b7134d768d59739289af1d4be93dc5d74a13
Author: Adrian Hunter <[email protected]>

i3c: master: Fix recursive locking during device registration

i3c_master_register_new_i3c_devs() registers newly discovered devices while holding i3c_bus_normaluse_lock(), leading to a recursive acquisition of the same rwsem. Fix this by separating device creation from device registration.

Quoted 4 lines

drivers/i3c/master.c +0 -0
index f485b98805cf6..d2fb1a1105218 100644

[ ... ]

drivers/i3c/master.c i3c_master_register_new_i3c_devs(struct i3c_master_controller *master) · line 2115

Quoted 4 lines

drivers/i3c/master.c +0 -0

@@ -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);

[Severity: High]
This is a pre-existing issue, but does device_set_node() take a reference to the fwnode here?

When an i3c boardinfo is added, a reference to the fwnode is acquired. However, it appears device_set_node() assigns this fwnode without incrementing the reference count. When the device is freed, the reference is unconditionally dropped:

drivers/i3c/master.c:i3c_device_release() {
    ...
    fwnode_handle_put(dev->fwnode);
    ...
}

If device_register() fails, put_device() will invoke i3c_device_release() and drop the fwnode reference. If the device is subsequently rediscovered, a new device is created with the same fwnode assigned, potentially leading to a refcount underflow and a use-after-free.

Quoted 5 lines

drivers/i3c/master.c +0 -5

-		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;

drivers/i3c/master.c i3c_master_register_new_i3c_devs(struct i3c_master_controller *master) · line 2122

Quoted 10 lines

drivers/i3c/master.c +7 -3

-			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);

[Severity: High]
Could dropping the bus lock around device_register() allow a concurrent dynamic address assignment operation to free the i3c device descriptor?

During device_register(), the driver core invokes i3c_device_uevent(), which accesses the descriptor without taking the bus lock:

drivers/i3c/master.c:i3c_device_uevent() {
    ...
    if (i3cdev->desc)
        devinfo = i3cdev->desc->info;
    ...
}

If a concurrent dynamic address assignment operation (like a hot-join) executes, it takes the bus lock in write mode. If it detects the device as a duplicate, it replaces the descriptor and frees the old one:

drivers/i3c/master.c:__i3c_master_add_i3c_dev_locked() {
    ...
    newdev->dev->desc = newdev;
    ...
    i3c_master_free_i3c_dev(olddev);
    ...
}

If i3c_device_uevent() fetches i3cdev->desc before the swap and dereferences it after the free, is it possible this leads to a use-after-free?

Quoted 5 lines

drivers/i3c/master.c +5 -0

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

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Adrian Hunter

author

2026-08-07 14:57 UTC #3 lore raw

PATCH v5 2/14 i3c: Fix unlocked dereference of dev->desc in i3c_device_get_supported_xfer_mode()

In reply to 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 [email protected]

Signed-off-by Adrian Hunter <[email protected]>

Reviewed-by Frank Li <[email protected]>

---

Changes in V5:

Added Frank's Rev'd-by

Changes in V4:

None

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(-)

drivers/i3c/device.c +1 -1

index 101eaa77de68..a3778282e84c 100644
@@ -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);

drivers/i3c/internals.h +5 -0

index 0f1f3f766623..86a36b951e0d 100644
@@ -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 */

drivers/i3c/master.c +0 -6

index d2fb1a110521..c7bb52b71d88 100644
@@ -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);

Signature · 1 line

2.53.0

[email protected]

AI review

2026-08-07 15:13 UTC #4 lore raw

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

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

Pre-existing issues:
- [High] Unresolved lockless dereferences of i3cdev->desc and uninitialized stack memory leaks to user-space.
- [High] I3C master controllers are allocated using devm_kzalloc while embedding a struct device, leading to a Use-After-Free during unbinding. --

--- Patch [2]: [PATCH V5 02/14] i3c: Fix unlocked dereference of dev->desc in i3c_device_get_supported_xfer_mode() --- commit e7d56c173e65ff36b79485393042fbeb29cba21d
Author: Adrian Hunter <[email protected]>

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

This patch resolves an unlocked dereference of dev->desc by using dev->bus to obtain the master controller instead. The commit notes that dev->desc must not be dereferenced unless bus->lock is held.

Quoted 3 lines

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.

[Severity: High]
This isn't a bug introduced by this patch, but since we are establishing that dev->desc must not be dereferenced without bus->lock, are there similar unresolved lockless dereferences elsewhere?

For example, looking at drivers/i3c/master.c:i3c_device_uevent():

	if (i3cdev->desc)
		devinfo = i3cdev->desc->info;
	manuf = I3C_PID_MANUF_ID(devinfo.pid);

If this races with descriptor freeing (such as during DAA/Hot-Join events that swap or free descriptors), could this cause a Use-After-Free?

Additionally, if i3cdev->desc is NULL, devinfo remains completely uninitialized on the stack. The function then unconditionally formats devinfo.pid and devinfo.dcr into the MODALIAS string, which is exposed to user-space via the uevent sysfs file. Could this leak kernel stack memory?

Similar lockless or NULL-unsafe accesses appear to exist in drivers/i3c/device.c:i3c_device_match_id(), drivers/i3c/master.c:i3c_device_match(), and driver probes like drivers/misc/amd-sbi/rmi-i2c.c:sbrmi_i3c_probe().

[ ... ]

Quoted 7 lines

drivers/i3c/master.c +0 -0

index d2fb1a1105218..c7bb52b71d884 100644
@@ -102,12 +102,6 @@ void i3c_bus_normaluse_unlock(struct i3c_bus *bus)
 	up_read(&bus->lock);
 }

drivers/i3c/master.c void i3c_bus_normaluse_unlock(struct i3c_bus *bus) · line 107

Quoted 10 lines

drivers/i3c/master.c +0 -6

 
-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);

[Severity: High]
This is a pre-existing issue, but looking at the master controller lifecycle managed here, is there a Use-After-Free risk during unbinding?

Almost all I3C master drivers (e.g. drivers/i3c/master/dw-i3c-master.c) allocate their master controller structure using devm_kzalloc():

drivers/i3c/master/dw-i3c-master.c:dw_i3c_probe() {
...

	master = devm_kzalloc(&pdev->dev, sizeof(*master), GFP_KERNEL);

...
}

This structure embeds a struct i3c_master_controller, which in turn embeds a struct device (accessed right here via dev_to_i3cmaster()).

When the driver is unbound, devres automatically frees the devm_kzalloc memory once the remove callback finishes. If a user holds a sysfs attribute open, the device reference count doesn't immediately drop to zero. Later, when the sysfs file is closed, the driver core invokes the device's release callback:

drivers/i3c/master.c:i3c_masterdev_release() {
...

	struct i3c_master_controller *master = dev_to_i3cmaster(dev);
...
	if (master->wq)
		destroy_workqueue(master->wq);

...
}

Since the memory was already freed by devres, won't accessing master->wq here trigger a Use-After-Free?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2

Adrian Hunter

author

2026-08-07 14:57 UTC #5 lore raw

PATCH v5 3/14 i3c: master: Do not treat master device as a duplicate target

In reply to 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 [email protected]

Signed-off-by Adrian Hunter <[email protected]>

Reviewed-by Frank Li <[email protected]>

Acked-by Mukesh Savaliya <[email protected]>

---

Changes in V5:

None

Changes in V4:

Added Frank's Rev-by and Mukesh's Acked-by tags

Changes in V3:

New patch

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

drivers/i3c/master.c +2 -1

index c7bb52b71d88..abb582645a2e 100644
@@ -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;
 	}

Signature · 1 line

2.53.0

[email protected]

AI review

2026-08-07 15:07 UTC #6 lore raw

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

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

Quoted 5 lines

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.

Quoted 7 lines

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 [email protected]
Signed-off-by Adrian Hunter <[email protected]>

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

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3

Adrian Hunter

author

2026-08-07 14:57 UTC #7 lore raw

PATCH v5 4/14 i3c: master: Fix use-after-free of master->this

In reply to Adrian Hunter

sysfs attribute callbacks for the master controller device dereference master->this. However, master->this is 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(), which is called after the master device and its sysfs state are being torn down. Do not free master->this as part of the normal device detach path.

On the error path in i3c_master_set_info(), reset master->this and bus.cur_master to NULL before freeing the allocated device.

Fixes 3a379bbcea0a ("i3c: Add core I3C infrastructure")

Cc [email protected]

Signed-off-by Adrian Hunter <[email protected]>

Reviewed-by Frank Li <[email protected]>

---

Changes in V5:

Added Frank's Rev'd-by

Changes in V4:

Also reset master->this and bus.cur_master to NULL on the i3c_master_set_info() error path before freeing the allocated device. Tidied up the commit message wording.

Changes in V3:

New patch

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

drivers/i3c/master.c +11 -6

index abb582645a2e..2357874bb9d6 100644
@@ -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)

@@ -2266,6 +2268,8 @@ int i3c_master_set_info(struct i3c_master_controller *master,
 	return 0;
 
 err_free_dev:
+	master->bus.cur_master = NULL;
+	master->this = NULL;
 	i3c_master_free_i3c_dev(i3cdev);
 
 	return ret;

@@ -2286,7 +2290,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,

Signature · 1 line

2.53.0

[email protected]

AI review

2026-08-07 15:14 UTC #8 lore raw

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

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

Pre-existing issues:
- [High] Pre-existing devres lifetime mismatch causing Use-After-Free in i3c_masterdev_release(), exacerbated by the patch's newly added kfree().
- [High] Pre-existing IDR state corruption due to missing error handling around i3cbus->id in probe failure path.
--

--- Patch [4]: [PATCH V5 04/14] i3c: master: Fix use-after-free of master->this --- commit aad15513dbf2d559bf893789d55a331f522383d1
Author: Adrian Hunter <[email protected]>
Subject: i3c: master: Fix use-after-free of master->this

The patch fixes a use-after-free by keeping master->this alive until i3c_masterdev_release(). It also resets master->this and bus.cur_master to NULL on the error path in i3c_master_set_info() before freeing the allocated device.

Cc [email protected]

Quoted 17 lines

drivers/i3c/master.c +5 -0

index abb582645a2ed..2357874bb9d6a 100644
@@ -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);

drivers/i3c/master.c static void i3c_masterdev_release(struct device *dev) · line 859

Quoted 1 line

drivers/i3c/master.c +0 -0

@@ -854,6 +859,8 @@ static void i3c_masterdev_release(struct device *dev)
 	i3c_bus_cleanup(bus);

[Severity: High]
This isn't a bug introduced by this patch, but does the error path in probe corrupt the IDR if idr_alloc() fails?

If idr_alloc() fails in i3c_bus_init(), i3cbus->id is not updated and remains at its kzalloc-initialized value of 0.

The error path in i3c_master_register() calls put_device(&master->dev), which invokes this i3c_masterdev_release() callback.

That unconditionally calls i3c_bus_cleanup(bus), which executes idr_remove(&i3c_bus_idr, i3cbus->id). This effectively deletes ID 0 from the IDR, even though that ID might belong to another successfully initialized bus.

drivers/i3c/master.c static void i3c_masterdev_release(struct device *dev) · line 863

Quoted 4 lines

drivers/i3c/master.c +2 -0

 	fwnode_handle_put(dev->fwnode);
+
+	i3c_master_free_i3c_dev(master->this);

[Severity: High]
This is a pre-existing issue, but can this devres lifetime mismatch lead to a use-after-free when sysfs files are held open?

The i3c_master_controller struct is typically allocated via devm_kzalloc() by the parent driver. If an unprivileged user holds a sysfs file open (like the mode or current_master attributes), device_unregister() drops a reference, but the sysfs node prevents the kobject from being released.

When the driver is unbound and remove() returns, devres frees the master struct.

Later, when the sysfs file is closed, the refcount drops to 0 and i3c_masterdev_release() executes, dereferencing the already-freed master struct to access master->wq, bus, and the newly added i3c_master_free_i3c_dev(master->this).

Quoted 1 line

drivers/i3c/master.c +0 -0
 }

[ ... ]

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4

Adrian Hunter

author

2026-08-07 14:57 UTC #9 lore raw

PATCH v5 5/14 i3c: Make dev->desc locking assumptions explicit

In reply to 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 <[email protected]>

Acked-by Mukesh Savaliya <[email protected]>

Reviewed-by Frank Li <[email protected]>

---

Changes in V5:

Added Frank's Rev'd-by

Changes in V4:

Added Mukesh's Acked-by tag

Changes in V3:

New patch

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

drivers/i3c/device.c +1 -2

index a3778282e84c..5e6df6de0283 100644
@@ -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);

Signature · 1 line

2.53.0

[email protected]

AI review

2026-08-07 15:20 UTC #10 lore raw

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

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

Pre-existing issues:
- [High] Use-After-Free and potential information leak in i3c_device_uevent() due to missing locks during concurrent device descriptor replacement. --

--- Patch [5]: [PATCH V5 05/14] i3c: Make dev->desc locking assumptions explicit --- commit 278282a9eadd6c355b60a12ba9af4c6b5982e144
Author: Adrian Hunter <[email protected]>

i3c: Make dev->desc locking assumptions explicit

This commit removes an unnecessary NULL pointer check in i3c_device_get_info() since the descriptor pointer is guaranteed valid under the bus normal-use lock.

drivers/i3c/device.c void i3c_device_get_info(const struct i3c_device *dev, · line 105

Quoted 12 lines

drivers/i3c/device.c +1 -2

index a3778282e84c2..5e6df6de02834 100644
@@ -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);

[Severity: High]
This is a pre-existing issue, but does the same locking rule need to be applied to i3c_device_uevent() in drivers/i3c/master.c?

It appears that i3c_device_uevent() currently reads i3cdev->desc without holding the normal-use bus lock:

drivers/i3c/master.c:i3c_device_uevent() {
        struct i3c_device_info devinfo;
        ...
        if (i3cdev->desc)
                devinfo = i3cdev->desc->info;
        ...
}

Could this sequence cause a use-after-free if a concurrent device discovery or hot-join occurs? For example, if __i3c_master_add_i3c_dev_locked() replaces and frees the descriptor while the device is still accessible via sysfs uevents:

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

Also, if i3cdev->desc evaluates to NULL during the uevent, could this leave the devinfo stack variable uninitialized and expose kernel stack memory to user space via the modalias string?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5

Adrian Hunter

author

2026-08-07 14:57 UTC #11 lore raw

PATCH v5 6/14 i3c: master: Fix potential UAF in i3c_device_uevent()

In reply to 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 [email protected] # requires "i3c: master: Fix recursive locking during device registration"

Signed-off-by Adrian Hunter <[email protected]>

Acked-by Mukesh Savaliya <[email protected]>

Reviewed-by Frank Li <[email protected]>

---

Changes in V5:

Added Frank's Rev'd-by

Changes in V4:

Added Mukesh's Acked-by tag

Changes in V3:

New patch

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

drivers/i3c/master.c +1 -2

index 2357874bb9d6..ee771ac0b203 100644
@@ -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);

Signature · 1 line

2.53.0

[email protected]

AI review

2026-08-07 15:21 UTC #12 lore raw

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

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

Quoted 13 lines

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.

Quoted 8 lines

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 [email protected] # requires "i3c: master: Fix recursive locking during device registration"
Signed-off-by Adrian Hunter <[email protected]>

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

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6

Adrian Hunter

author

2026-08-07 14:57 UTC #13 lore raw

PATCH v5 7/14 i3c: master: Fix potential UAF in i3c_device_match()

In reply to 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 <[email protected]>

---

Changes in V4 and V5:

None

Changes in V3:

New patch

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

drivers/i3c/master.c +3 -1

index ee771ac0b203..83e04a2c202a 100644
@@ -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

Signature · 1 line

2.53.0

[email protected]

AI review

2026-08-07 15:37 UTC #14 lore raw

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

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

New issues:
- [High] The removal of the i3cdev->desc NULL check introduces a deterministic NULL pointer dereference in i3c_device_match during concurrent driver binding.

Pre-existing issues:
- [High] Incomplete fix: A Use-After-Free of the descriptor is still possible during master unregistration because it executes locklessly. --

--- Patch [7]: [PATCH V5 07/14] i3c: master: Fix potential UAF in i3c_device_match() --- commit 8aa5de79b60445da0abee6c6a17c70b1b9ec8be9
Author: Adrian Hunter <[email protected]>

i3c: master: Fix potential UAF in i3c_device_match()

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.

drivers/i3c/master.c static int i3c_device_match(struct device *dev, const struct device_driver *drv) · line 351

Quoted 11 lines

drivers/i3c/master.c +2 -1

index ee771ac0b203e..83e04a2c202a9 100644
@@ -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)

[Severity: High]
Can removing this NULL check lead to a NULL pointer dereference during concurrent driver binding?

Looking at master unregistration, the descriptor pointer is cleared locklessly:

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

If a concurrent sysfs bind operation triggers i3c_device_match(), it might acquire the read lock but still observe a NULL descriptor since unregistration does not acquire i3c_bus_normaluse_lock(). Does the removed NULL check now guarantee a crash in this race condition?

[Severity: High]
This is a pre-existing issue, but does taking the normal-use lock fully prevent the use-after-free this patch aims to fix?

Master unregistration modifies and initiates cleanup of descriptors without acquiring any bus locks, bypassing this protection entirely:

drivers/i3c/master.c:i3c_master_bus_cleanup() {
        ...
        i3c_master_detach_free_devs(master);
        ...
}

If a concurrent sysfs bind operation reads i3cdev->desc just before it is cleared, the read lock here doesn't seem to serialize against the unregistration thread proceeding to free the descriptor. Can the descriptor still be freed while i3c_device_match() is dereferencing it?

Quoted 5 lines

drivers/i3c/master.c +1 -0

 		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

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7

Frank Li

Reviewed-by on v5 7/14

2026-08-07 19:43 UTC #15 lore raw

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

In reply to Adrian Hunter

Quoted 10 lines · 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

Remove "This change", just depend on ...

Quoted 5 lines

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".

It should be fixes tag, but this patch still not in linus tree yet, not sure how alex handle this.

Alex:
Maybe rebase i3c tree,

move "i3c: master: Fix recursive locking during device" before "i3c: master: match I3C device through DT and ACPI"

Then squash this into "i3c: master: match I3C device through DT and ACPI"

Or simple ignore "this fixes ..." sentense.

Reviewed-by Frank Li <[email protected]>

Frank

Quoted 36 lines

Signed-off-by Adrian Hunter <[email protected]>

---

Changes in V4 and V5:

None

Changes in V3:

New patch

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

drivers/i3c/master.c +3 -2

index ee771ac0b203..83e04a2c202a 100644
@@ -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

Alexandre Belloni

maintainer

2026-08-08 13:08 UTC #16 lore raw

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

Quoted 14 lines · Frank Li

Quoted 10 lines · 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
Remove "This change", just depend on ...

Quoted 9 lines

Quoted 5 lines
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".
It should be fixes tag, but this patch still not in linus tree yet, not sure how alex handle this.

I've added the Fixes tag, this works as long as the tree isn't rebased before send the PR to Linus.

Quoted 55 lines

Alex:
Maybe rebase i3c tree,
move "i3c: master: Fix recursive locking during device" before "i3c: master: match I3C device through DT and ACPI"
Then squash this into "i3c: master: match I3C device through DT and ACPI"
Or simple ignore "this fixes ..." sentense.
Reviewed-by Frank Li <[email protected]>
Frank
Quoted 36 lines
Signed-off-by Adrian Hunter <[email protected]>
---
Changes in V4 and V5:
None
Changes in V3:
New patch
 drivers/i3c/master.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/i3c/master.c +3 -2
index ee771ac0b203..83e04a2c202a 100644
@@ -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
Signature · 3 lines
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Signature · 3 lines

Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Adrian Hunter

author

2026-08-07 14:57 UTC #17 lore raw

PATCH v5 8/14 i3c: master: Support IBI-based wakeup capability

In reply to 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 <[email protected]>

Reviewed-by Frank Li <[email protected]>

Acked-by Mukesh Savaliya <[email protected]>

---

Changes in V5:

Added Mukesh's Ack'd-by

Changes in V4:

Shortened the comment about IBI-capable devices being wakeup capable.

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       | 4 ++++
 include/linux/i3c/master.h | 2 ++
 2 files changed, 6 insertions(+)

drivers/i3c/master.c +4 -0

index 83e04a2c202a..2616354dbe54 100644
@@ -2110,6 +2110,10 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
 		if (desc->boardinfo)
 			device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);
 
+		/* If the device has IBI capability, set as wakeup capable */
+		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);
 	}

include/linux/i3c/master.h +2 -0

index 26535beb1e77..9d675d01522c 100644
@@ -524,6 +524,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

@@ -563,6 +564,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;

Signature · 1 line

2.53.0

[email protected]

AI review

2026-08-07 15:16 UTC #18 lore raw

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

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

Quoted 5 lines

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.

Quoted 8 lines

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 <[email protected]>

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

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8

Adrian Hunter

author

2026-08-07 14:57 UTC #19 lore raw

PATCH v5 9/14 i3c: master: Report wakeup events for IBIs

In reply to 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 <[email protected]>

Reviewed-by Frank Li <[email protected]>

---

Changes in V5:

Added Frank's Rev'd-by

Changes in V4:

None

Changes in V2 and V3:

None

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

drivers/i3c/master.c +6 -0

index 2616354dbe54..6c5341491944 100644
@@ -3384,6 +3384,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

@@ -3397,6 +3400,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);
 }

Signature · 1 line

2.53.0

[email protected]

AI review

2026-08-07 15:41 UTC #20 lore raw

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

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

New issues:
- [High] Unprotected access to dev->dev in i3c_master_queue_ibi() introduces a NULL pointer dereference or Use-After-Free during concurrent device teardown.

Pre-existing issues:
- [High] Preexisting lockless access to dev->ibi in i3c_master_queue_ibi() leads to a Use-After-Free (UAF) if an IBI is handled concurrently with IBI freeing. --

--- Patch [9]: [PATCH V5 09/14] i3c: master: Report wakeup events for IBIs --- commit b0c07417134eb0c8209460a9c252faf745d3feee
Author: Adrian Hunter <[email protected]>
Subject: i3c: master: Report wakeup events for IBIs

This commit modifies the I3C master subsystem to report wakeup events for devices configured as wakeup sources. It queues a wakeup event when an In-Band Interrupt (IBI) is processed by i3c_master_queue_ibi().

Quoted 4 lines

drivers/i3c/master.c +0 -0
index 2616354dbe545..6c53414919445 100644

[ ... ]

drivers/i3c/master.c void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot) · line 3405

Quoted 7 lines

drivers/i3c/master.c +3 -0

@@ -3397,6 +3400,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]
Does this code introduce a potential NULL pointer dereference or use-after-free if a concurrent teardown unregisters the device?

Since i3c_master_queue_ibi() runs locklessly from the master's IRQ handler, could a concurrent teardown thread reach i3c_master_unregister_i3c_devs():

	put_device(&i3cdev->dev->dev);
	i3cdev->dev = NULL;

while the IRQ handler is between the !dev->ibi check and this new wakeup check?

If the controller's disable_ibi callback does not synchronize with the IRQ handler using synchronize_irq(), the teardown thread could see pending_ibis == 0 and proceed to unbind the device, which would cause dereferencing dev->dev->dev here to panic.

drivers/i3c/master.c void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot) · line 3407

Quoted 2 lines

drivers/i3c/master.c +0 -0

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

[Severity: High]
This is a pre-existing issue, but can this lockless access to dev->ibi cause a use-after-free during concurrent device unbinding?

If the IRQ handler has passed the !dev->ibi check but hasn't yet reached this atomic_inc(), a concurrent thread executing i3c_dev_free_ibi_locked() could disable IBIs.

Because pending_ibis has not been incremented yet, it would see pending_ibis == 0 and proceed to free the IBI resources without waiting:

	kfree(dev->ibi);
	dev->ibi = NULL;

When the IRQ handler resumes, it would execute atomic_inc() and queue_work(), writing to the freed dev->ibi structure.

Quoted 1 line

drivers/i3c/master.c +0 -0
 }

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9

Adrian Hunter

author

2026-08-07 14:57 UTC #21 lore raw

PATCH v5 10/14 i3c: master: Add helper to query bus wakeup requirements

In reply to Adrian Hunter

Add i3c_master_has_wakeup_enabled_devs(), 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 <[email protected]>

Reviewed-by Frank Li <[email protected]>

Acked-by Mukesh Savaliya <[email protected]>

---

Changes in V5:

Added Frank's Rev'd-by
Added Mukesh's Ack'd-by

Changes in V4:

Renamed i3c_master_any_wakeup_enabled() to i3c_master_has_wakeup_enabled_devs().

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(+)

drivers/i3c/master.c +35 -0

index 6c5341491944..afcd7a21a3e6 100644
@@ -2147,6 +2147,41 @@ static void i3c_master_reg_work_fn(struct work_struct *work)
 	i3c_master_register_new_i3c_devs(master);
 }
 
+/**
+ * i3c_master_has_wakeup_enabled_devs() - 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_has_wakeup_enabled_devs(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_has_wakeup_enabled_devs);
+
 /**
  * i3c_master_dma_map_single() - Map buffer for single DMA transfer
  * @dev: device object of a device doing DMA

include/linux/i3c/master.h +1 -0

index 9d675d01522c..82d9886e7f12 100644
@@ -765,6 +765,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_has_wakeup_enabled_devs(struct i3c_master_controller *master);
 
 struct i3c_ibi_slot *i3c_master_get_free_ibi_slot(struct i3c_dev_desc *dev);

Signature · 1 line

2.53.0

[email protected]

AI review

2026-08-07 15:21 UTC #22 lore raw

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

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

Quoted 8 lines

Add i3c_master_has_wakeup_enabled_devs(), 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 <[email protected]>

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

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=10

Adrian Hunter

author

2026-08-07 14:57 UTC #23 lore raw

PATCH v5 11/14 i3c: master: Reject IBI requests from non-IBI-capable devices

In reply to 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 <[email protected]>

Reviewed-by Frank Li <[email protected]>

---

Changes in V4 and V5:

None

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(-)

drivers/i3c/device.c +5 -3

index 5e6df6de0283..f1ba363b22a1 100644
@@ -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);

Signature · 1 line

2.53.0

[email protected]

AI review

2026-08-07 15:26 UTC #24 lore raw

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

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

Pre-existing issues:
- [High] Workqueue memory and rescuer thread leak on the error path of i3c_dev_request_ibi_locked. --

--- Patch [11]: [PATCH V5 11/14] i3c: master: Reject IBI requests from non-IBI-capable devices --- commit d6c55123f731ff15725d89d2f9b3fef0f879efa9
Author: Adrian Hunter <[email protected]>

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

This patch checks if a device advertises IBI support before attempting to set up IBI handling. It adds a check for I3C_BCR_IBI_REQ_CAP in i3c_device_request_ibi() and returns -EOPNOTSUPP if not supported.

Quoted 3 lines

drivers/i3c/device.c +0 -0

[ ... ]

drivers/i3c/device.c int i3c_device_request_ibi(struct i3c_device *dev, · line 213

Quoted 12 lines

drivers/i3c/device.c +5 -1

@@ -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 looking at the error path of i3c_dev_request_ibi_locked() in drivers/i3c/master.c, does it leak the workqueue and its rescuer thread if the hardware driver fails the request?

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

	return ret;
}

When a device driver requests an IBI and the bus controller fails the request, the ibi struct is freed but destroy_workqueue() is never called. Since the workqueue is created with WQ_MEM_RECLAIM, wouldn't this leak a kernel task and its associated memory on every failure?

Quoted 5 lines

drivers/i3c/device.c +0 -2

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

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11

Adrian Hunter

author

2026-08-07 14:57 UTC #25 lore raw

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

In reply to 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 <[email protected]>

Reviewed-by Frank Li <[email protected]>

---

Changes in V5:

Added Frank's Rev'd-by

Changes in V4:

Updated for the rename of i3c_master_any_wakeup_enabled() to i3c_master_has_wakeup_enabled_devs().

Changes in V2 and V3:

None

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

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +21 -2

index 5a9e2a43eff8..42c172e82622 100644
@@ -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_has_wakeup_enabled_devs(&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)

Signature · 1 line

2.53.0

[email protected]

AI review

2026-08-07 15:38 UTC #26 lore raw

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

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

New issues:
- [High] AB-BA deadlock between I3C subsystem locks (bus.lock / ibi_lock) and PCI runtime suspend.
- [Medium] Dynamically modifying user-controlled sysfs wakeup attribute during suspend.

Pre-existing issues:
- [High] Missing child driver bound check and PM API bypass leads to NULL pointer dereferences. --

--- Patch [12]: [PATCH V5 12/14] i3c: mipi-i3c-hci-pci: Propagate I3C wakeup requirements to PCI --- commit 84ca2ce77fe5f7e3e2fb196059be4ac7c80842fc
Author: Adrian Hunter <[email protected]>
Subject: i3c: mipi-i3c-hci-pci: Propagate I3C wakeup requirements to PCI

This patch aligns the PCI controller's wakeup state with the wakeup requirements of its attached I3C devices. During suspend, it enables PCI wakeup only if at least one I3C device has wakeup enabled.

Quoted 4 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +0 -0
index 5a9e2a43eff8d..42c172e82622f 100644

[ ... ]

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c static bool mipi_i3c_hci_pci_is_mfd(struct device *dev) · line 283

Quoted 11 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +7 -0

@@ -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_has_wakeup_enabled_devs(&hci->master);
+}
+

[Severity: High]
This is a pre-existing issue, but does this code dereference the driver data without checking if it is NULL?

If the child mipi-i3c-hci driver fails to probe or is unbound by a user via sysfs, dev_get_drvdata() returns NULL.

The parent PCI driver iterates over child devices during suspend and checks if the hardware is running:

mipi_i3c_hci_pci_suspend_instance()

    if (!mipi_i3c_hci_pci_is_mfd(dev) ||
        !mipi_i3c_hci_pci_is_operational(dev, true))
        return 0;

Because the hardware can still be operational even if unbound, the code proceeds to call i3c_hci_rpm_suspend() and now mipi_i3c_hci_pci_any_wakeup_enabled(), both of which blindly dereference the unverified driver data.

Quoted 2 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +0 -0
 static int mipi_i3c_hci_pci_suspend_instance(struct device *dev, void *data)
 {

[ ... ]

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c static int mipi_i3c_hci_pci_suspend_instance(struct device *dev, void *data) · line 301

Quoted 7 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +3 -0

@@ -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;
+

[Severity: High]
Is there a potential AB-BA deadlock between the I3C subsystem locks and the PCI runtime suspend lock here?

The new i3c_master_has_wakeup_enabled_devs() function is called during the PCI device's suspend, acquiring bus.lock and ibi_lock:

mipi_i3c_hci_pci_suspend_instance()
mipi_i3c_hci_pci_any_wakeup_enabled()

    i3c_master_has_wakeup_enabled_devs()
      i3c_bus_normaluse_lock(&master->bus);
      guard(mutex)(&desc->ibi_lock);

However, other I3C core paths take these locks first before triggering a PM operation that requires the PM lock:

i3c_device_free_ibi()
i3c_bus_normaluse_lock(dev->bus);
mutex_lock(&dev->desc->ibi_lock);

  i3c_dev_free_ibi_locked(dev->desc);
    ...
    i3c_master_rpm_get()
      pm_runtime_resume_and_get()

If an IBI is freed concurrently while the PCI parent enters autosuspend, one thread holds bus.lock waiting for the PM lock, while the suspend path holds the PM lock waiting for bus.lock.

Quoted 11 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +2 -1

 	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)

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c static int mipi_i3c_hci_pci_suspend(struct device *dev) · line 343

Quoted 10 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +6 -1

+	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);
+

[Severity: Medium]
Does this directly overwrite the user-controlled sysfs wakeup attribute during suspend?

The official driver API guidelines mandate that device drivers should not call device_set_wakeup_enable() directly, as this field is reserved for sysfs user policy.

Should the driver align its hardware PME registers directly without modifying the generic PM subsystem's sysfs toggle?

Quoted 2 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +1 -0
+	return 0;
 }

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=12

Adrian Hunter

author

2026-08-07 14:57 UTC #27 lore raw

PATCH v5 13/14 i3c: mipi-i3c-hci: Factor out i3c_hci_sysdev()

In reply to 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.

The explanatory comment moves with the code, reworked as kernel-doc now that it documents a function rather than an inline block.

Signed-off-by Adrian Hunter <[email protected]>

Reviewed-by Frank Li <[email protected]>

Acked-by Mukesh Savaliya <[email protected]>

---

Changes in V5:

Added Frank's Rev'd-by
Added Mukesh's Ack'd-by

Changes in V4:

Reworked the comment that moves with the code into kernel-doc, and mentioned that in the commit message.

Changes in V2 and V3:

None

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

drivers/i3c/master/mipi-i3c-hci/core.c +16 -0

index cfe9b5390b56..f95da427d7a9 100644
@@ -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,21 @@ static inline struct i3c_hci *to_i3c_hci(struct i3c_master_controller *m)
 	return container_of(m, struct i3c_hci, master);
 }
 
+/**
+ * i3c_hci_sysdev() - Get the device to use for DMA
+ * @dev: Device the HCI controller is bound to
+ *
+ * When an IOMMU is enabled, DMA API calls must use the device that IOMMU
+ * setup was done for.  Under PCI enumeration that is the PCI device, not
+ * the "mipi-i3c-hci" platform device below it.
+ *
+ * Return: @dev's parent if it is a PCI device, otherwise @dev.
+ */
+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,

drivers/i3c/master/mipi-i3c-hci/dma.c +1 ~~-14~~

index 0672ed1132f8..7c2b20474130 100644
@@ -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));

drivers/i3c/master/mipi-i3c-hci/hci.h +2 -0

index b3d9803b1968..b8d2a3d680f8 100644
@@ -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);

Signature · 1 line

2.53.0

[email protected]

AI review

2026-08-07 15:40 UTC #28 lore raw

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

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

Quoted 11 lines

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.
The explanatory comment moves with the code, reworked as kernel-doc now that it documents a function rather than an inline block.
Signed-off-by Adrian Hunter <[email protected]>

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

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=13

Adrian Hunter

author

2026-08-07 14:57 UTC #29 lore raw

PATCH v5 14/14 i3c: mipi-i3c-hci: Advertise IBI wakeup capability

In reply to 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 <[email protected]>

Reviewed-by Frank Li <[email protected]>

---

Changes in V5:

None

Changes in V4:

Updated the i3c_hci_sysdev() kernel-doc to mention system PM and wakeup, in place of the old comment tweak.

Changes in V3:

Added Frank's Rev-by tag

Changes in V2:

None

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

drivers/i3c/master/mipi-i3c-hci/core.c +6 -2

index f95da427d7a9..06acf6cf5a0d 100644
@@ -119,12 +119,13 @@ static inline struct i3c_hci *to_i3c_hci(struct i3c_master_controller *m)
 }
 
 /**
- * i3c_hci_sysdev() - Get the device to use for DMA
+ * i3c_hci_sysdev() - Get the device to use for DMA and system PM
  * @dev: Device the HCI controller is bound to
  *
  * When an IOMMU is enabled, DMA API calls must use the device that IOMMU
  * setup was done for.  Under PCI enumeration that is the PCI device, not
- * the "mipi-i3c-hci" platform device below it.
+ * the "mipi-i3c-hci" platform device below it.  The same device owns
+ * system PM and wakeup configuration.
  *
  * Return: @dev's parent if it is a PCI device, otherwise @dev.
  */

@@ -1180,6 +1181,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);
 }

Signature · 1 line

2.53.0

[email protected]

AI review

2026-08-07 15:30 UTC #30 lore raw

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

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

Quoted 7 lines

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 <[email protected]>

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

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=14

Alexandre Belloni

maintainer Applied

2026-08-08 13:10 UTC #31 lore raw

In reply to Adrian Hunter

Quoted 10 lines · Adrian Hunter

Note: all patches except patch 7 now have Frank's Rev'd-by
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.
[...]

Applied, thanks!

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

        https://git.kernel.org/i3c/c/456f832e5fc2
[02/14] i3c: Fix unlocked dereference of dev->desc in i3c_device_get_supported_xfer_mode()
        https://git.kernel.org/i3c/c/8bed7f4fa710
[03/14] i3c: master: Do not treat master device as a duplicate target
        https://git.kernel.org/i3c/c/4dc1b3eeba79
[04/14] i3c: master: Fix use-after-free of master->this
        https://git.kernel.org/i3c/c/feb0ed76601f
[05/14] i3c: Make dev->desc locking assumptions explicit
        https://git.kernel.org/i3c/c/4083d192f6f3
[06/14] i3c: master: Fix potential UAF in i3c_device_uevent()
        https://git.kernel.org/i3c/c/e5e8dd2e959f
[07/14] i3c: master: Fix potential UAF in i3c_device_match()
        https://git.kernel.org/i3c/c/f44d3b15326c
[08/14] i3c: master: Support IBI-based wakeup capability
        https://git.kernel.org/i3c/c/9fd18a865591
[09/14] i3c: master: Report wakeup events for IBIs
        https://git.kernel.org/i3c/c/ff2b20f8f1ff
[10/14] i3c: master: Add helper to query bus wakeup requirements
        https://git.kernel.org/i3c/c/60ff731f0690
[11/14] i3c: master: Reject IBI requests from non-IBI-capable devices
        https://git.kernel.org/i3c/c/087efc8e88e2
[12/14] i3c: mipi-i3c-hci-pci: Propagate I3C wakeup requirements to PCI
        https://git.kernel.org/i3c/c/94da8edf849b
[13/14] i3c: mipi-i3c-hci: Factor out i3c_hci_sysdev()
        https://git.kernel.org/i3c/c/842e46925ef9
[14/14] i3c: mipi-i3c-hci: Advertise IBI wakeup capability
        https://git.kernel.org/i3c/c/1421e948a32a

Best regards,

Signature · 3 lines

Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com