From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 4A5C51094471 for ; Sat, 21 Mar 2026 12:41:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=Gdw96O2s3CiuyvCHVeb+6oKXS/Dbg2GEk36YWsEx2Do=; b=cezwU1yey8YoKqOBLcEbij6voU Np/wJTtEatJKEEqDsty7Yybhd2JlFAdhFII4RkwzCJffszQvDBjdYXXNm87hOQpeWdszc4k/cHN4h 6hUBvZ804OUuyEi1yweDpb0ryJXC/+irX+NoGgYeKtZ+qM3TTOcxYo6f1vQeUc6/MxHmrmaQ33Iyk OuOK6vIGdy+a+JEaBMuoxrmhZfzYEaGyvYP6eIqwzeJDOzQQdIS1clA35Z3iSLNGW1k27+JFneiYl +FOKc6ZEPmtoldgJMQRbd1pWdRRyE/kMVw09ODcWj6wMf5wE0PNQUAoRnRb+atTh/G1ypJ9j6B/GR 0ldHfT9w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1w3vdg-0000000ESkk-2vLh; Sat, 21 Mar 2026 12:41:12 +0000 Received: from tor.source.kernel.org ([2600:3c04:e001:324:0:1991:8:25]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1w3vde-0000000ESk8-20Fw; Sat, 21 Mar 2026 12:41:10 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by tor.source.kernel.org (Postfix) with ESMTP id 64206600AE; Sat, 21 Mar 2026 12:41:09 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 437D2C19421; Sat, 21 Mar 2026 12:41:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1774096868; bh=fxVoXze93TK9b7JkjLP3hYYoZwJ+sQWedrY/aS+Uubs=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=rjUxIz20bGymV7Rs5Wvfoj/C3PHimtOj2oswMcvSb4/xuT2XsaNg2m8Au0INjBZ0e 5NM8fJbWPfZsIZzcfUwGrKHNO4D9xBdzsH5kuHJwzsUfPhN2T4Ait6dR0XKwSm/Jvl urLpexwJz9ruvDCpqst42b6CcRcbz/39Xk3KYwNbDw3TywC7YsgMqpZEXXsjGYHalR FJznFWd8EFZ2wcIzrW7+Y0a7YFd1k6y7zbIvuglzgHWH/0trij0rO5Zvdvkos+R9QO VPp8eKim71vTs8gkcR0883dGJesTBzPct5hoGDaYrEMWMMVkdN679j0AE+938q7CQN 9DS9D5pstZoTQ== Date: Sat, 21 Mar 2026 13:41:06 +0100 From: Lorenzo Bianconi To: Jakub Kicinski Cc: Andrew Lunn , "David S. Miller" , Eric Dumazet , Paolo Abeni , Simon Horman , linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, netdev@vger.kernel.org Subject: Re: [PATCH net-next] net: airoha: Reset PPE cpu port configuration in airoha_ppe_hw_init() Message-ID: References: <20260317-airoha-fix-ppe-def-cpu-v1-1-338533d8e234@kernel.org> <20260320183127.22b360be@kernel.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="3sTRF6na1p3lBxUs" Content-Disposition: inline In-Reply-To: <20260320183127.22b360be@kernel.org> X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org --3sTRF6na1p3lBxUs Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable > On Tue, 17 Mar 2026 17:40:47 +0100 Lorenzo Bianconi wrote: > > @@ -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); >=20 > AI says: >=20 > Can this lead to a NULL pointer dereference if a port is not fully > initialized? >=20 > 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. >=20 > During airoha_register_gdm_devices(), the ports are registered sequential= ly > with register_netdev(). Since register_netdev() drops the rtnl_lock(), > userspace could react to the RTM_NEWLINK event of the first registered po= rt > and apply a tc flow offload rule. >=20 > This would trigger the following call chain: > .ndo_setup_tc() -> airoha_ppe_setup_tc_block_cb() -> airoha_ppe_offload= _setup()=20 > -> airoha_ppe_hw_init() >=20 > If airoha_ppe_hw_init() iterates over the array, it will find the subsequ= ent > port that has been allocated but not yet registered, meaning its port->qd= ma > is still NULL. The call to airoha_ppe_set_cpu_port(port, i) will then > dereference the NULL port->qdma. >=20 > 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/etherne= t/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, =20 static int airoha_register_gdm_devices(struct airoha_eth *eth) { - int i; + int i, err =3D 0; + + mutex_lock(&flow_offload_mutex); =20 for (i =3D 0; i < ARRAY_SIZE(eth->ports); i++) { struct airoha_gdm_port *port =3D eth->ports[i]; - int err; =20 if (!port) continue; =20 err =3D register_netdev(port->dev); if (err) - return err; + break; } =20 - return 0; + mutex_unlock(&flow_offload_mutex); + + return err; } =20 static int airoha_probe(struct platform_device *pdev) diff --git a/drivers/net/ethernet/airoha/airoha_eth.h b/drivers/net/etherne= t/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) =20 +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/etherne= t/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" =20 -static DEFINE_MUTEX(flow_offload_mutex); +DEFINE_MUTEX(flow_offload_mutex); static DEFINE_SPINLOCK(ppe_lock); =20 static const struct rhashtable_params airoha_flow_table_params =3D { What do you think? I noticed commit f44218cd5e6a is already applied to net-next. I will post a follow-up patch, agree? Regards, Lorenzo > --=20 > pw-bot: cr --3sTRF6na1p3lBxUs Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iHUEABYKAB0WIQTquNwa3Txd3rGGn7Y6cBh0uS2trAUCab6R4gAKCRA6cBh0uS2t rAo2AP0VgofFC019vKtsTfxIA2WGT+rQR//Zf0DpJyAuqtpUWAD9EsknoKEO2zlx 9aN6ois6lbevq4CS+K7KKEGrqFIfAAs= =tUik -----END PGP SIGNATURE----- --3sTRF6na1p3lBxUs--