On Tue, 17 Mar 2026 17:40:47 +0100 Lorenzo Bianconi wrote:

quoted

@@ -155,6 +171,11 @@ static void airoha_ppe_hw_init(struct airoha_ppe *ppe)
 						 AIROHA_MAX_MTU) |
 				      FIELD_PREP(FP1_EGRESS_MTU_MASK,
 						 AIROHA_MAX_MTU));
+			if (!port)
+				continue;
+
+			airoha_ppe_set_cpu_port(port, i);

AI says:

Can this lead to a NULL pointer dereference if a port is not fully
initialized?

In airoha_probe(), all GDM ports defined in the device tree are allocated
and the eth->ports[] array is populated with pointers, but port->qdma is
left as NULL.

During airoha_register_gdm_devices(), the ports are registered sequentially
with register_netdev(). Since register_netdev() drops the rtnl_lock(),
userspace could react to the RTM_NEWLINK event of the first registered port
and apply a tc flow offload rule.

This would trigger the following call chain:
  .ndo_setup_tc() -> airoha_ppe_setup_tc_block_cb() -> airoha_ppe_offload_setup() 
   -> airoha_ppe_hw_init()

If airoha_ppe_hw_init() iterates over the array, it will find the subsequent
port that has been allocated but not yet registered, meaning its port->qdma
is still NULL. The call to airoha_ppe_set_cpu_port(port, i) will then
dereference the NULL port->qdma.

Would it be better to check if (!port || !port->qdma) before calling
airoha_ppe_set_cpu_port()?

Hi Jakub,

I agree there is a race between airoha_register_gdm_devices() and
airoha_ppe_setup_tc_block_cb(). Instead of not running
airoha_ppe_set_cpu_port() in airoha_ppe_hw_init() for a given port and not
configuring the hw correctly, I guess it would be better to grab
flow_offload_mutex mutex in airoha_register_gdm_devices routine and wait for
all devices to be properly registered before executing
airoha_ppe_setup_tc_block_cb(). Something like:

diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
index 961325da833b..f5bb917f0f6e 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.c
+++ b/drivers/net/ethernet/airoha/airoha_eth.c

@@ -2927,21 +2927,24 @@ static int airoha_alloc_gdm_port(struct airoha_eth *eth,
 
 static int airoha_register_gdm_devices(struct airoha_eth *eth)
 {
-	int i;
+	int i, err = 0;
+
+	mutex_lock(&flow_offload_mutex);
 
 	for (i = 0; i < ARRAY_SIZE(eth->ports); i++) {
 		struct airoha_gdm_port *port = eth->ports[i];
-		int err;
 
 		if (!port)
 			continue;
 
 		err = register_netdev(port->dev);
 		if (err)
-			return err;
+			break;
 	}
 
-	return 0;
+	mutex_unlock(&flow_offload_mutex);
+
+	return err;
 }
 
 static int airoha_probe(struct platform_device *pdev)

diff --git a/drivers/net/ethernet/airoha/airoha_eth.h b/drivers/net/ethernet/airoha/airoha_eth.h
index 7df4dbcd8861..3330ee54e20e 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.h
+++ b/drivers/net/ethernet/airoha/airoha_eth.h

@@ -551,6 +551,8 @@ struct airoha_gdm_port {
 #define AIROHA_RXD4_PPE_CPU_REASON	GENMASK(20, 16)
 #define AIROHA_RXD4_FOE_ENTRY		GENMASK(15, 0)
 
+extern struct mutex flow_offload_mutex;
+
 struct airoha_ppe {
 	struct airoha_ppe_dev dev;
 	struct airoha_eth *eth;

diff --git a/drivers/net/ethernet/airoha/airoha_ppe.c b/drivers/net/ethernet/airoha/airoha_ppe.c
index 7666b1d2f4f6..96d092f7214e 100644
--- a/drivers/net/ethernet/airoha/airoha_ppe.c
+++ b/drivers/net/ethernet/airoha/airoha_ppe.c

@@ -15,7 +15,7 @@
 #include "airoha_regs.h"
 #include "airoha_eth.h"
 
-static DEFINE_MUTEX(flow_offload_mutex);
+DEFINE_MUTEX(flow_offload_mutex);
 static DEFINE_SPINLOCK(ppe_lock);
 
 static const struct rhashtable_params airoha_flow_table_params = {

What do you think?
I noticed commit f44218cd5e6a is already applied to net-next. I will post a
follow-up patch, agree?

Regards,
Lorenzo

-- 
pw-bot: cr