public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/11] firmware: arm_ffa: Fix cleanup, notification, and discovery paths
@ 2026-04-28 18:33 Sudeep Holla
  2026-04-28 18:33 ` [PATCH v2 01/11] firmware: arm_ffa: Check for NULL FF-A ID table while driver registration Sudeep Holla
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: Sudeep Holla @ 2026-04-28 18:33 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Jens Wiklander, Sudeep Holla, Sebastian Ene

Hi all,

This series fixes a set of issues in the FF-A driver around init
cleanup, framework notification handling, v1.0 notifier lifetime, and
partition discovery.

The fixes are all small and localized, but together they tighten a few
important paths:

- fix the early init unwind path when RX buffer allocation fails
- align the stored RX/TX buffer size with the size actually mapped to
  firmware
- ensure the framework notification handler always releases the RX
  buffer correctly
- validate framework notification payload bounds before copying data out
  of the shared RX buffer
- fix the partition lookup used for sched-recv callback registration
- unregister the FF-A v1.0 bus notifier during teardown
- bound the register-based partition discovery copies against the caller
  buffer
- reject FF-A driver registration without an ID table
- fix per-vcpu drivers own notifications handling in workqueue
- make NPI per cpu work item to avoid collapsing already queued work

This is the outcome of the self-initiated review of the entire driver
following the oversight of Sashiko’s review on one of the patches that
was merged.

https://sashiko.dev/#/patchset/20260402113939.930221-1-sebastianene@google.com

Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
Changes in v2:
- Addressed valid comments from sashiko
  (https://sashiko.dev/#/patchset/20260423-ffa_fixes-v1-0-61189661affe@kernel.org)
- Added couple of new fixes based on the review
- Link to v1: https://patch.msgid.link/20260423-ffa_fixes-v1-0-61189661affe@kernel.org

---
Sudeep Holla (11):
      firmware: arm_ffa: Check for NULL FF-A ID table while driver registration
      firmware: arm_ffa: Skip free_pages on RX buffer alloc failure
      firmware: arm_ffa: Avoid collapsing NPI work from different CPUs
      firmware: arm_ffa: Fix per-vcpu self notifications handling in workqueue
      firmware: arm_ffa: Unregister bus notifier on teardown for FF-A v1.0
      firmware: arm_ffa: Bound PARTITION_INFO_GET_REGS copies
      firmware: arm_ffa: Keep framework RX release under lock
      firmware: arm_ffa: Validate framework notification message layout
      firmware: arm_ffa: Align RxTx buffer size before mapping
      firmware: arm_ffa: Snapshot notifier callbacks under lock
      firmware: arm_ffa: Fix sched-recv callback partition lookup

 drivers/firmware/arm_ffa/bus.c    |   4 +-
 drivers/firmware/arm_ffa/driver.c | 138 +++++++++++++++++++++++++++-----------
 2 files changed, 102 insertions(+), 40 deletions(-)
---
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
change-id: 20260423-ffa_fixes-4ad33f0ee250


-- 
Regards,
Sudeep


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 01/11] firmware: arm_ffa: Check for NULL FF-A ID table while driver registration
  2026-04-28 18:33 [PATCH v2 00/11] firmware: arm_ffa: Fix cleanup, notification, and discovery paths Sudeep Holla
@ 2026-04-28 18:33 ` Sudeep Holla
  2026-04-28 18:33 ` [PATCH v2 02/11] firmware: arm_ffa: Skip free_pages on RX buffer alloc failure Sudeep Holla
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Sudeep Holla @ 2026-04-28 18:33 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel; +Cc: Jens Wiklander, Sudeep Holla

The bus match callback assumes that every FF-A driver provides an
id_table and dereferences it unconditionally. Enforce that contract at
registration time so a buggy client driver cannot crash the bus during
match.

Fixes: 92743071464f ("firmware: arm_ffa: Ensure drivers provide a probe function")
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
 drivers/firmware/arm_ffa/bus.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/arm_ffa/bus.c b/drivers/firmware/arm_ffa/bus.c
index 9576862d89c4..601c3418e0d9 100644
--- a/drivers/firmware/arm_ffa/bus.c
+++ b/drivers/firmware/arm_ffa/bus.c
@@ -26,6 +26,8 @@ static int ffa_device_match(struct device *dev, const struct device_driver *drv)
 
 	id_table = to_ffa_driver(drv)->id_table;
 	ffa_dev = to_ffa_dev(dev);
+	if (!id_table)
+		return 0;
 
 	while (!uuid_is_null(&id_table->uuid)) {
 		/*
@@ -123,7 +125,7 @@ int ffa_driver_register(struct ffa_driver *driver, struct module *owner,
 {
 	int ret;
 
-	if (!driver->probe)
+	if (!driver->probe || !driver->id_table)
 		return -EINVAL;
 
 	driver->driver.bus = &ffa_bus_type;

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 02/11] firmware: arm_ffa: Skip free_pages on RX buffer alloc failure
  2026-04-28 18:33 [PATCH v2 00/11] firmware: arm_ffa: Fix cleanup, notification, and discovery paths Sudeep Holla
  2026-04-28 18:33 ` [PATCH v2 01/11] firmware: arm_ffa: Check for NULL FF-A ID table while driver registration Sudeep Holla
@ 2026-04-28 18:33 ` Sudeep Holla
  2026-04-28 18:33 ` [PATCH v2 03/11] firmware: arm_ffa: Avoid collapsing NPI work from different CPUs Sudeep Holla
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Sudeep Holla @ 2026-04-28 18:33 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel; +Cc: Jens Wiklander, Sudeep Holla

If the RX buffer allocation fails in ffa_init(), the error path jumps to
free_pages even though no buffer has been allocated yet. Route that case
directly to free_drv_info so the cleanup path is only used after at
least one RX/TX buffer allocation has succeeded.

Fixes: 3bbfe9871005 ("firmware: arm_ffa: Add initial Arm FFA driver support")
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
 drivers/firmware/arm_ffa/driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index eb2782848283..e6a051b20cb7 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -2067,7 +2067,7 @@ static int __init ffa_init(void)
 	drv_info->rx_buffer = alloc_pages_exact(rxtx_bufsz, GFP_KERNEL);
 	if (!drv_info->rx_buffer) {
 		ret = -ENOMEM;
-		goto free_pages;
+		goto free_drv_info;
 	}
 
 	drv_info->tx_buffer = alloc_pages_exact(rxtx_bufsz, GFP_KERNEL);

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 03/11] firmware: arm_ffa: Avoid collapsing NPI work from different CPUs
  2026-04-28 18:33 [PATCH v2 00/11] firmware: arm_ffa: Fix cleanup, notification, and discovery paths Sudeep Holla
  2026-04-28 18:33 ` [PATCH v2 01/11] firmware: arm_ffa: Check for NULL FF-A ID table while driver registration Sudeep Holla
  2026-04-28 18:33 ` [PATCH v2 02/11] firmware: arm_ffa: Skip free_pages on RX buffer alloc failure Sudeep Holla
@ 2026-04-28 18:33 ` Sudeep Holla
  2026-04-28 18:33 ` [PATCH v2 04/11] firmware: arm_ffa: Fix per-vcpu self notifications handling in workqueue Sudeep Holla
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Sudeep Holla @ 2026-04-28 18:33 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel; +Cc: Jens Wiklander, Sudeep Holla

Notification pending interrupts are registered as per-CPU IRQs, but the
driver queues all NPI handling through a single shared work_struct.

That allows queue_work_on() calls from different CPUs to collapse onto a
single pending work item even though the work function uses the CPU it
runs on to fetch and handle per-CPU notifications.

Move notif_pcpu_work into the per-CPU ffa_pcpu_irq state and initialize
one work item per CPU. This keeps NPI handling independent per CPU and
avoids losing notifications when multiple CPUs queue work concurrently.

Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
 drivers/firmware/arm_ffa/driver.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index e6a051b20cb7..4e66c7325a4e 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -87,6 +87,7 @@ static inline int ffa_to_linux_errno(int errno)
 
 struct ffa_pcpu_irq {
 	struct ffa_drv_info *info;
+	struct work_struct notif_pcpu_work;
 };
 
 struct ffa_drv_info {
@@ -106,7 +107,6 @@ struct ffa_drv_info {
 	unsigned int cpuhp_state;
 	struct ffa_pcpu_irq __percpu *irq_pcpu;
 	struct workqueue_struct *notif_pcpu_wq;
-	struct work_struct notif_pcpu_work;
 	struct work_struct sched_recv_irq_work;
 	struct xarray partition_info;
 	DECLARE_HASHTABLE(notifier_hash, ilog2(FFA_MAX_NOTIFICATIONS));
@@ -1539,8 +1539,9 @@ ffa_self_notif_handle(u16 vcpu, bool is_per_vcpu, void *cb_data)
 
 static void notif_pcpu_irq_work_fn(struct work_struct *work)
 {
-	struct ffa_drv_info *info = container_of(work, struct ffa_drv_info,
+	struct ffa_pcpu_irq *pcpu = container_of(work, struct ffa_pcpu_irq,
 						 notif_pcpu_work);
+	struct ffa_drv_info *info = pcpu->info;
 
 	ffa_self_notif_handle(smp_processor_id(), true, info);
 }
@@ -1811,7 +1812,7 @@ static irqreturn_t notif_pend_irq_handler(int irq, void *irq_data)
 	struct ffa_drv_info *info = pcpu->info;
 
 	queue_work_on(smp_processor_id(), info->notif_pcpu_wq,
-		      &info->notif_pcpu_work);
+		      &pcpu->notif_pcpu_work);
 
 	return IRQ_HANDLED;
 }
@@ -1928,8 +1929,11 @@ static int ffa_init_pcpu_irq(void)
 	if (!irq_pcpu)
 		return -ENOMEM;
 
-	for_each_present_cpu(cpu)
+	for_each_present_cpu(cpu) {
 		per_cpu_ptr(irq_pcpu, cpu)->info = drv_info;
+		INIT_WORK(&per_cpu_ptr(irq_pcpu, cpu)->notif_pcpu_work,
+			  notif_pcpu_irq_work_fn);
+	}
 
 	drv_info->irq_pcpu = irq_pcpu;
 
@@ -1958,7 +1962,6 @@ static int ffa_init_pcpu_irq(void)
 	}
 
 	INIT_WORK(&drv_info->sched_recv_irq_work, ffa_sched_recv_irq_work_fn);
-	INIT_WORK(&drv_info->notif_pcpu_work, notif_pcpu_irq_work_fn);
 	drv_info->notif_pcpu_wq = create_workqueue("ffa_pcpu_irq_notification");
 	if (!drv_info->notif_pcpu_wq)
 		return -EINVAL;

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 04/11] firmware: arm_ffa: Fix per-vcpu self notifications handling in workqueue
  2026-04-28 18:33 [PATCH v2 00/11] firmware: arm_ffa: Fix cleanup, notification, and discovery paths Sudeep Holla
                   ` (2 preceding siblings ...)
  2026-04-28 18:33 ` [PATCH v2 03/11] firmware: arm_ffa: Avoid collapsing NPI work from different CPUs Sudeep Holla
@ 2026-04-28 18:33 ` Sudeep Holla
  2026-04-28 18:33 ` [PATCH v2 05/11] firmware: arm_ffa: Unregister bus notifier on teardown for FF-A v1.0 Sudeep Holla
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Sudeep Holla @ 2026-04-28 18:33 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel; +Cc: Jens Wiklander, Sudeep Holla

Per-vcpu notification handling already runs from a per-cpu work item on
the target cpu. Routing that path back through smp_call_function_single()
re-enters the call-function IPI path and executes the notification
handler with interrupts disabled. That makes the framework path unsafe,
since it takes a mutex, allocates memory with GFP_KERNEL, and invokes
client callbacks.

Handle per-vcpu self notifications directly from the existing per-cpu
work item instead. This keeps the per-vcpu path in task context and
avoids the extra IPI hop entirely.

Fixes: 3a3e2b83e805 ("firmware: arm_ffa: Avoid queuing work when running on the worker queue")
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
 drivers/firmware/arm_ffa/driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index 4e66c7325a4e..2241e851f7ae 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -1543,7 +1543,7 @@ static void notif_pcpu_irq_work_fn(struct work_struct *work)
 						 notif_pcpu_work);
 	struct ffa_drv_info *info = pcpu->info;
 
-	ffa_self_notif_handle(smp_processor_id(), true, info);
+	notif_get_and_handle(info);
 }
 
 static const struct ffa_info_ops ffa_drv_info_ops = {

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 05/11] firmware: arm_ffa: Unregister bus notifier on teardown for FF-A v1.0
  2026-04-28 18:33 [PATCH v2 00/11] firmware: arm_ffa: Fix cleanup, notification, and discovery paths Sudeep Holla
                   ` (3 preceding siblings ...)
  2026-04-28 18:33 ` [PATCH v2 04/11] firmware: arm_ffa: Fix per-vcpu self notifications handling in workqueue Sudeep Holla
@ 2026-04-28 18:33 ` Sudeep Holla
  2026-04-28 18:33 ` [PATCH v2 06/11] firmware: arm_ffa: Bound PARTITION_INFO_GET_REGS copies Sudeep Holla
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Sudeep Holla @ 2026-04-28 18:33 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel; +Cc: Jens Wiklander, Sudeep Holla

For FF-A v1.0 the driver registers a bus notifier to backfill UUID
matching, but the notifier was never unregistered on cleanup paths.
Track the registration state and unregister it during teardown and early
partition-setup failure.

Fixes: 9dd15934f60d ("firmware: arm_ffa: Move the FF-A v1.0 NULL UUID workaround to bus notifier")
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
 drivers/firmware/arm_ffa/driver.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index 2241e851f7ae..a122814eb6d7 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -101,6 +101,7 @@ struct ffa_drv_info {
 	bool mem_ops_native;
 	bool msg_direct_req2_supp;
 	bool bitmap_created;
+	bool bus_notifier_registered;
 	bool notif_enabled;
 	unsigned int sched_recv_irq;
 	unsigned int notif_pend_irq;
@@ -1630,6 +1631,15 @@ static struct notifier_block ffa_bus_nb = {
 	.notifier_call = ffa_bus_notifier,
 };
 
+static void ffa_bus_notifier_unregister(void)
+{
+	if (!drv_info->bus_notifier_registered)
+		return;
+
+	bus_unregister_notifier(&ffa_bus_type, &ffa_bus_nb);
+	drv_info->bus_notifier_registered = false;
+}
+
 static int ffa_xa_add_partition_info(struct ffa_device *dev)
 {
 	struct ffa_dev_part_info *info;
@@ -1713,6 +1723,8 @@ static void ffa_partitions_cleanup(void)
 	struct list_head *phead;
 	unsigned long idx;
 
+	ffa_bus_notifier_unregister();
+
 	/* Clean up/free all registered devices */
 	ffa_devices_unregister();
 
@@ -1740,11 +1752,14 @@ static int ffa_setup_partitions(void)
 		ret = bus_register_notifier(&ffa_bus_type, &ffa_bus_nb);
 		if (ret)
 			pr_err("Failed to register FF-A bus notifiers\n");
+		else
+			drv_info->bus_notifier_registered = true;
 	}
 
 	count = ffa_partition_probe(&uuid_null, &pbuf);
 	if (count <= 0) {
 		pr_info("%s: No partitions found, error %d\n", __func__, count);
+		ffa_bus_notifier_unregister();
 		return -EINVAL;
 	}
 

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 06/11] firmware: arm_ffa: Bound PARTITION_INFO_GET_REGS copies
  2026-04-28 18:33 [PATCH v2 00/11] firmware: arm_ffa: Fix cleanup, notification, and discovery paths Sudeep Holla
                   ` (4 preceding siblings ...)
  2026-04-28 18:33 ` [PATCH v2 05/11] firmware: arm_ffa: Unregister bus notifier on teardown for FF-A v1.0 Sudeep Holla
@ 2026-04-28 18:33 ` Sudeep Holla
  2026-04-28 18:33 ` [PATCH v2 07/11] firmware: arm_ffa: Keep framework RX release under lock Sudeep Holla
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Sudeep Holla @ 2026-04-28 18:33 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel; +Cc: Jens Wiklander, Sudeep Holla

The register-based PARTITION_INFO_GET path trusted the firmware-provided
indices when copying partition descriptors into the caller buffer.
Reject inconsistent counts or index progressions so the copy loop cannot
write past the allocated array.

Fixes: ba85c644ac8d ("firmware: arm_ffa: Add support for FFA_PARTITION_INFO_GET_REGS")
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
 drivers/firmware/arm_ffa/driver.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index a122814eb6d7..ed502486eb35 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -323,6 +323,12 @@ __ffa_partition_info_get(u32 uuid0, u32 uuid1, u32 uuid2, u32 uuid3,
 #define PART_INFO_ID_MASK	GENMASK(15, 0)
 #define PART_INFO_EXEC_CXT_MASK	GENMASK(31, 16)
 #define PART_INFO_PROPS_MASK	GENMASK(63, 32)
+#define FFA_PART_INFO_GET_REGS_FIRST_REG	3
+#define FFA_PART_INFO_GET_REGS_REGS_PER_DESC	3
+#define FFA_PART_INFO_GET_REGS_MAX_DESC \
+	(((sizeof(ffa_value_t) / sizeof_field(ffa_value_t, a0)) - \
+	  FFA_PART_INFO_GET_REGS_FIRST_REG) / \
+	 FFA_PART_INFO_GET_REGS_REGS_PER_DESC)
 #define PART_INFO_ID(x)		((u16)(FIELD_GET(PART_INFO_ID_MASK, (x))))
 #define PART_INFO_EXEC_CXT(x)	((u16)(FIELD_GET(PART_INFO_EXEC_CXT_MASK, (x))))
 #define PART_INFO_PROPERTIES(x)	((u32)(FIELD_GET(PART_INFO_PROPS_MASK, (x))))
@@ -336,7 +342,7 @@ __ffa_partition_info_get_regs(u32 uuid0, u32 uuid1, u32 uuid2, u32 uuid3,
 
 	do {
 		__le64 *regs;
-		int idx;
+		int idx, nr_desc, buf_idx;
 
 		start_idx = prev_idx ? prev_idx + 1 : 0;
 
@@ -354,15 +360,28 @@ __ffa_partition_info_get_regs(u32 uuid0, u32 uuid1, u32 uuid2, u32 uuid3,
 			count = PARTITION_COUNT(partition_info.a2);
 		if (!buffer || !num_parts) /* count only */
 			return count;
+		if (count > num_parts)
+			return -EINVAL;
 
 		cur_idx = CURRENT_INDEX(partition_info.a2);
+		if (cur_idx < start_idx || cur_idx >= count)
+			return -EINVAL;
+
+		nr_desc = cur_idx - start_idx + 1;
+		if (nr_desc > FFA_PART_INFO_GET_REGS_MAX_DESC)
+			return -EINVAL;
+
+		buf_idx = buf - buffer;
+		if (buf_idx + nr_desc > num_parts)
+			return -EINVAL;
+
 		tag = UUID_INFO_TAG(partition_info.a2);
 		buf_sz = PARTITION_INFO_SZ(partition_info.a2);
 		if (buf_sz > sizeof(*buffer))
 			buf_sz = sizeof(*buffer);
 
 		regs = (void *)&partition_info.a3;
-		for (idx = 0; idx < cur_idx - start_idx + 1; idx++, buf++) {
+		for (idx = 0; idx < nr_desc; idx++, buf++) {
 			union {
 				uuid_t uuid;
 				u64 regs[2];

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 07/11] firmware: arm_ffa: Keep framework RX release under lock
  2026-04-28 18:33 [PATCH v2 00/11] firmware: arm_ffa: Fix cleanup, notification, and discovery paths Sudeep Holla
                   ` (5 preceding siblings ...)
  2026-04-28 18:33 ` [PATCH v2 06/11] firmware: arm_ffa: Bound PARTITION_INFO_GET_REGS copies Sudeep Holla
@ 2026-04-28 18:33 ` Sudeep Holla
  2026-04-28 18:33 ` [PATCH v2 08/11] firmware: arm_ffa: Validate framework notification message layout Sudeep Holla
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Sudeep Holla @ 2026-04-28 18:33 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel; +Cc: Jens Wiklander, Sudeep Holla

The framework notification handler drops rx_lock before issuing
FFA_RX_RELEASE, leaving a window where another RX-buffer user can
start a new FF-A transaction before ownership has actually been
returned to firmware.

Move the FFA_RX_RELEASE calls so they execute while rx_lock is still
held on both the kmemdup() failure path and the normal success path.
While doing that, switch the handler to scoped_guard() to keep the
critical section explicit.

Fixes: 285a5ea0f542 ("firmware: arm_ffa: Add support for handling framework notifications")
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
 drivers/firmware/arm_ffa/driver.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index ed502486eb35..18bcbd161805 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -1494,25 +1494,22 @@ static void handle_fwk_notif_callbacks(u32 bitmap)
 	if (!(bitmap & FRAMEWORK_NOTIFY_RX_BUFFER_FULL))
 		return;
 
-	mutex_lock(&drv_info->rx_lock);
+	scoped_guard(mutex, &drv_info->rx_lock) {
+		msg = drv_info->rx_buffer;
+		buf = kmemdup((void *)msg + msg->offset, msg->size, GFP_KERNEL);
+		if (!buf) {
+			ffa_rx_release();
+			return;
+		}
 
-	msg = drv_info->rx_buffer;
-	buf = kmemdup((void *)msg + msg->offset, msg->size, GFP_KERNEL);
-	if (!buf) {
-		mutex_unlock(&drv_info->rx_lock);
-		return;
+		target = SENDER_ID(msg->send_recv_id);
+		if (msg->offset >= sizeof(*msg))
+			uuid_copy(&uuid, &msg->uuid);
+		else
+			uuid_copy(&uuid, &uuid_null);
+		ffa_rx_release();
 	}
 
-	target = SENDER_ID(msg->send_recv_id);
-	if (msg->offset >= sizeof(*msg))
-		uuid_copy(&uuid, &msg->uuid);
-	else
-		uuid_copy(&uuid, &uuid_null);
-
-	mutex_unlock(&drv_info->rx_lock);
-
-	ffa_rx_release();
-
 	read_lock(&drv_info->notify_lock);
 	cb_info = notifier_hnode_get_by_vmid_uuid(notify_id, target, &uuid);
 	read_unlock(&drv_info->notify_lock);

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 08/11] firmware: arm_ffa: Validate framework notification message layout
  2026-04-28 18:33 [PATCH v2 00/11] firmware: arm_ffa: Fix cleanup, notification, and discovery paths Sudeep Holla
                   ` (6 preceding siblings ...)
  2026-04-28 18:33 ` [PATCH v2 07/11] firmware: arm_ffa: Keep framework RX release under lock Sudeep Holla
@ 2026-04-28 18:33 ` Sudeep Holla
  2026-04-28 18:33 ` [PATCH v2 09/11] firmware: arm_ffa: Align RxTx buffer size before mapping Sudeep Holla
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Sudeep Holla @ 2026-04-28 18:33 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel; +Cc: Jens Wiklander, Sudeep Holla

Framework notifications carry an indirect message in the shared RX
buffer. Validate the reported offset and size before using them, reject
zero-length payloads, and ensure that any non-header payload starts at
the UUID field rather than in the middle of the message header.

Use the validated offset and size values for both kmemdup() and the UUID
parsing path so malformed firmware data cannot drive an out-of-bounds
read or an oversized allocation.

Fixes: 285a5ea0f542 ("firmware: arm_ffa: Add support for handling framework notifications")
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
 drivers/firmware/arm_ffa/driver.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index 18bcbd161805..4944aa6b815f 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -1489,21 +1489,35 @@ static void handle_fwk_notif_callbacks(u32 bitmap)
 	int notify_id = 0, target;
 	struct ffa_indirect_msg_hdr *msg;
 	struct notifier_cb_info *cb_info = NULL;
+	size_t min_offset = offsetof(struct ffa_indirect_msg_hdr, uuid);
 
 	/* Only one framework notification defined and supported for now */
 	if (!(bitmap & FRAMEWORK_NOTIFY_RX_BUFFER_FULL))
 		return;
 
 	scoped_guard(mutex, &drv_info->rx_lock) {
+		u32 offset, size;
+
 		msg = drv_info->rx_buffer;
-		buf = kmemdup((void *)msg + msg->offset, msg->size, GFP_KERNEL);
+		offset = msg->offset;
+		size = msg->size;
+
+		if (!size || (offset != min_offset && offset < sizeof(*msg)) ||
+		    offset > drv_info->rxtx_bufsz ||
+		    size > drv_info->rxtx_bufsz - offset) {
+			pr_err("invalid framework notification message\n");
+			ffa_rx_release();
+			return;
+		}
+
+		buf = kmemdup((void *)msg + offset, size, GFP_KERNEL);
 		if (!buf) {
 			ffa_rx_release();
 			return;
 		}
 
 		target = SENDER_ID(msg->send_recv_id);
-		if (msg->offset >= sizeof(*msg))
+		if (offset >= sizeof(*msg))
 			uuid_copy(&uuid, &msg->uuid);
 		else
 			uuid_copy(&uuid, &uuid_null);

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 09/11] firmware: arm_ffa: Align RxTx buffer size before mapping
  2026-04-28 18:33 [PATCH v2 00/11] firmware: arm_ffa: Fix cleanup, notification, and discovery paths Sudeep Holla
                   ` (7 preceding siblings ...)
  2026-04-28 18:33 ` [PATCH v2 08/11] firmware: arm_ffa: Validate framework notification message layout Sudeep Holla
@ 2026-04-28 18:33 ` Sudeep Holla
  2026-04-28 18:33 ` [PATCH v2 10/11] firmware: arm_ffa: Snapshot notifier callbacks under lock Sudeep Holla
  2026-04-28 18:33 ` [PATCH v2 11/11] firmware: arm_ffa: Fix sched-recv callback partition lookup Sudeep Holla
  10 siblings, 0 replies; 12+ messages in thread
From: Sudeep Holla @ 2026-04-28 18:33 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Jens Wiklander, Sudeep Holla, Sebastian Ene

Commit 83210251fd70 ("firmware: arm_ffa: Use the correct buffer size during
RXTX_MAP") advertises PAGE_ALIGN(rxtx_bufsz) to firmware when mapping the
buffers but the driver continues to stores the minimum FF-A buffer size
in drv_info->rxtx_bufsz which is used elsewhere in the driver.

Align the size before storing it so that the allocation, validation and
FFA_RXTX_MAP all use the same buffer size.

Fixes: 83210251fd70 ("firmware: arm_ffa: Use the correct buffer size during RXTX_MAP")
Cc: Sebastian Ene <sebastianene@google.com>
Link: https://sashiko.dev/#/patchset/20260402113939.930221-1-sebastianene@google.com
Reviewed-by: Sebastian Ene <sebastianene@google.com>
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
 drivers/firmware/arm_ffa/driver.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index 4944aa6b815f..9181cc752ce1 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -2111,6 +2111,7 @@ static int __init ffa_init(void)
 			rxtx_bufsz = SZ_4K;
 	}
 
+	rxtx_bufsz = PAGE_ALIGN(rxtx_bufsz);
 	drv_info->rxtx_bufsz = rxtx_bufsz;
 	drv_info->rx_buffer = alloc_pages_exact(rxtx_bufsz, GFP_KERNEL);
 	if (!drv_info->rx_buffer) {
@@ -2126,7 +2127,7 @@ static int __init ffa_init(void)
 
 	ret = ffa_rxtx_map(virt_to_phys(drv_info->tx_buffer),
 			   virt_to_phys(drv_info->rx_buffer),
-			   PAGE_ALIGN(rxtx_bufsz) / FFA_PAGE_SIZE);
+			   rxtx_bufsz / FFA_PAGE_SIZE);
 	if (ret) {
 		pr_err("failed to register FFA RxTx buffers\n");
 		goto free_pages;

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 10/11] firmware: arm_ffa: Snapshot notifier callbacks under lock
  2026-04-28 18:33 [PATCH v2 00/11] firmware: arm_ffa: Fix cleanup, notification, and discovery paths Sudeep Holla
                   ` (8 preceding siblings ...)
  2026-04-28 18:33 ` [PATCH v2 09/11] firmware: arm_ffa: Align RxTx buffer size before mapping Sudeep Holla
@ 2026-04-28 18:33 ` Sudeep Holla
  2026-04-28 18:33 ` [PATCH v2 11/11] firmware: arm_ffa: Fix sched-recv callback partition lookup Sudeep Holla
  10 siblings, 0 replies; 12+ messages in thread
From: Sudeep Holla @ 2026-04-28 18:33 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel; +Cc: Jens Wiklander, Sudeep Holla

Both notification handlers currently look up a notifier callback under
notify_lock, drop the lock, and then dereference the returned
notifier entry. A concurrent unregister can delete and free that
entry in the gap, leaving the handler to dereference stale memory.

Copy the callback pointer and callback data while notify_lock is
still held and invoke the callback only after the lock is dropped.
This keeps the existing callback execution model while removing the
use-after-free window in both the framework and non-framework
notification paths.

Fixes: 285a5ea0f542 ("firmware: arm_ffa: Add support for handling framework notifications")
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
 drivers/firmware/arm_ffa/driver.c | 35 +++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index 9181cc752ce1..2e9820395162 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -1465,20 +1465,25 @@ static int ffa_notify_send(struct ffa_device *dev, int notify_id,
 
 static void handle_notif_callbacks(u64 bitmap, enum notify_type type)
 {
+	ffa_notifier_cb cb;
+	void *cb_data;
 	int notify_id;
-	struct notifier_cb_info *cb_info = NULL;
 
 	for (notify_id = 0; notify_id <= FFA_MAX_NOTIFICATIONS && bitmap;
 	     notify_id++, bitmap >>= 1) {
 		if (!(bitmap & 1))
 			continue;
 
-		read_lock(&drv_info->notify_lock);
-		cb_info = notifier_hnode_get_by_type(notify_id, type);
-		read_unlock(&drv_info->notify_lock);
+		scoped_guard(read_lock, &drv_info->notify_lock) {
+			struct notifier_cb_info *cb_info;
+
+			cb_info = notifier_hnode_get_by_type(notify_id, type);
+			cb = cb_info ? cb_info->cb : NULL;
+			cb_data = cb_info ? cb_info->cb_data : NULL;
+		}
 
-		if (cb_info && cb_info->cb)
-			cb_info->cb(notify_id, cb_info->cb_data);
+		if (cb)
+			cb(notify_id, cb_data);
 	}
 }
 
@@ -1486,9 +1491,10 @@ static void handle_fwk_notif_callbacks(u32 bitmap)
 {
 	void *buf;
 	uuid_t uuid;
+	void *fwk_cb_data;
 	int notify_id = 0, target;
+	ffa_fwk_notifier_cb fwk_cb;
 	struct ffa_indirect_msg_hdr *msg;
-	struct notifier_cb_info *cb_info = NULL;
 	size_t min_offset = offsetof(struct ffa_indirect_msg_hdr, uuid);
 
 	/* Only one framework notification defined and supported for now */
@@ -1524,12 +1530,17 @@ static void handle_fwk_notif_callbacks(u32 bitmap)
 		ffa_rx_release();
 	}
 
-	read_lock(&drv_info->notify_lock);
-	cb_info = notifier_hnode_get_by_vmid_uuid(notify_id, target, &uuid);
-	read_unlock(&drv_info->notify_lock);
+	scoped_guard(read_lock, &drv_info->notify_lock) {
+		struct notifier_cb_info *cb_info;
+
+		cb_info = notifier_hnode_get_by_vmid_uuid(notify_id, target,
+							  &uuid);
+		fwk_cb = cb_info ? cb_info->fwk_cb : NULL;
+		fwk_cb_data = cb_info ? cb_info->cb_data : NULL;
+	}
 
-	if (cb_info && cb_info->fwk_cb)
-		cb_info->fwk_cb(notify_id, cb_info->cb_data, buf);
+	if (fwk_cb)
+		fwk_cb(notify_id, fwk_cb_data, buf);
 	kfree(buf);
 }
 

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 11/11] firmware: arm_ffa: Fix sched-recv callback partition lookup
  2026-04-28 18:33 [PATCH v2 00/11] firmware: arm_ffa: Fix cleanup, notification, and discovery paths Sudeep Holla
                   ` (9 preceding siblings ...)
  2026-04-28 18:33 ` [PATCH v2 10/11] firmware: arm_ffa: Snapshot notifier callbacks under lock Sudeep Holla
@ 2026-04-28 18:33 ` Sudeep Holla
  10 siblings, 0 replies; 12+ messages in thread
From: Sudeep Holla @ 2026-04-28 18:33 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel; +Cc: Jens Wiklander, Sudeep Holla

ffa_sched_recv_cb_update() used list_for_each_entry_safe() to search for
a matching partition and then tested the iterator against NULL. That is
not a valid end-of-list check for circular lists and can fall through
with an invalid pointer. Use a normal iterator and detect the not-found
case correctly before touching the partition state.

Fixes: be61da938576 ("firmware: arm_ffa: Allow multiple UUIDs per partition to register SRI callback")
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
 drivers/firmware/arm_ffa/driver.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index 2e9820395162..7bf8555c09da 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -1209,7 +1209,7 @@ static int
 ffa_sched_recv_cb_update(struct ffa_device *dev, ffa_sched_recv_cb callback,
 			 void *cb_data, bool is_registration)
 {
-	struct ffa_dev_part_info *partition = NULL, *tmp;
+	struct ffa_dev_part_info *partition = NULL;
 	struct list_head *phead;
 	bool cb_valid;
 
@@ -1222,11 +1222,11 @@ ffa_sched_recv_cb_update(struct ffa_device *dev, ffa_sched_recv_cb callback,
 		return -EINVAL;
 	}
 
-	list_for_each_entry_safe(partition, tmp, phead, node)
+	list_for_each_entry(partition, phead, node)
 		if (partition->dev == dev)
 			break;
 
-	if (!partition) {
+	if (&partition->node == phead) {
 		pr_err("%s: No such partition ID 0x%x\n", __func__, dev->vm_id);
 		return -EINVAL;
 	}

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-04-28 18:34 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-28 18:33 [PATCH v2 00/11] firmware: arm_ffa: Fix cleanup, notification, and discovery paths Sudeep Holla
2026-04-28 18:33 ` [PATCH v2 01/11] firmware: arm_ffa: Check for NULL FF-A ID table while driver registration Sudeep Holla
2026-04-28 18:33 ` [PATCH v2 02/11] firmware: arm_ffa: Skip free_pages on RX buffer alloc failure Sudeep Holla
2026-04-28 18:33 ` [PATCH v2 03/11] firmware: arm_ffa: Avoid collapsing NPI work from different CPUs Sudeep Holla
2026-04-28 18:33 ` [PATCH v2 04/11] firmware: arm_ffa: Fix per-vcpu self notifications handling in workqueue Sudeep Holla
2026-04-28 18:33 ` [PATCH v2 05/11] firmware: arm_ffa: Unregister bus notifier on teardown for FF-A v1.0 Sudeep Holla
2026-04-28 18:33 ` [PATCH v2 06/11] firmware: arm_ffa: Bound PARTITION_INFO_GET_REGS copies Sudeep Holla
2026-04-28 18:33 ` [PATCH v2 07/11] firmware: arm_ffa: Keep framework RX release under lock Sudeep Holla
2026-04-28 18:33 ` [PATCH v2 08/11] firmware: arm_ffa: Validate framework notification message layout Sudeep Holla
2026-04-28 18:33 ` [PATCH v2 09/11] firmware: arm_ffa: Align RxTx buffer size before mapping Sudeep Holla
2026-04-28 18:33 ` [PATCH v2 10/11] firmware: arm_ffa: Snapshot notifier callbacks under lock Sudeep Holla
2026-04-28 18:33 ` [PATCH v2 11/11] firmware: arm_ffa: Fix sched-recv callback partition lookup Sudeep Holla

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox