[PATCH net v3 0/4] net: wan: fix FSL UCC HDLC lifecycle bugs

[PATCH net v3 0/4] net: wan: fix FSL UCC HDLC lifecycle bugs

[Myeonghun Pak]

This issue was identified during our ongoing static-analysis research while
reviewing kernel code.

The v2 review found two pre-existing FSL UCC HDLC bugs and asked that
the generic HDLC behavior change be separated and audited.

Patch 1 validates that an HDLC protocol is attached before starting the
UCC, avoiding a NULL protocol dereference on a premature interface open.
Patch 2 allocates the suspend backup before disabling NAPI, so allocation
failure leaves the running interface usable. Patch 3 closes active HDLC
devices before detaching their protocol and documents the audit of every
current caller, including the farsync probe unwind. Patch 4 unregisters
and frees the FSL UCC HDLC netdev before its private resources.

Patch 4 depends on patch 3 to stop an active device while its protocol is
still attached and before UCC and DMA resources are released.

These fixes and changelogs were prepared with an LLM and then checked
against the cited source paths and call ordering.

Changes since v2:
- split the generic unregister_hdlc_device() change into its own patch;
- audit all eight in-tree HDLC hardware drivers and document farsync;
- fix the NULL protocol dereference reported in uhdlc_open();
- keep NAPI enabled when the suspend backup allocation fails;
- rebase the series onto Linux 7.3-rc1.

v2: https://lore.kernel.org/r/20260803133048.42650-1-mhun512@gmail.com
review: https://lore.kernel.org/r/20260806020541.2011936-2-kuba@kernel.org

Validation:
The series applies cleanly to net commit 784450234395.
All four patches pass strict checkpatch.
No hardware runtime testing was performed.

Myeonghun Pak (4):
  net: wan: fsl_ucc_hdlc: validate protocol before starting device
  net: wan: fsl_ucc_hdlc: allocate suspend backup before quiescing
  net: wan: hdlc: close active devices before protocol detach
  net: wan: fsl_ucc_hdlc: release HDLC device on remove

 drivers/net/wan/fsl_ucc_hdlc.c | 29 ++++++++++++++++-------------
 drivers/net/wan/hdlc.c         |  1 +
 2 files changed, 17 insertions(+), 13 deletions(-)


base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
-- 
2.47.1

[PATCH net v3 1/4] net: wan: fsl_ucc_hdlc: validate protocol before starting device

[Myeonghun Pak]

uhdlc_open() starts the UCC, IRQ and NAPI before it calls hdlc_open().
If no HDLC protocol has been attached, hdlc_open() returns -ENOSYS. The
error path then calls uhdlc_close(), which calls hdlc_close() and
dereferences hdlc->proto even though it is NULL. Bringing up a freshly
registered interface before an IF_PROTO ioctl can therefore trigger a
NULL pointer dereference.

Call hdlc_open() before enabling the hardware. Balance a successful
protocol open with hdlc_close() if requesting the IRQ then fails. This
matches peer HDLC drivers and avoids running teardown for a protocol that
never opened.

Fixes: a59addacf899 ("drivers/net: process the result of hdlc_open() and add call of hdlc_close() in uhdlc_close()")
Cc: stable@vger.kernel.org
Reported-by: Jakub Kicinski <kuba@kernel.org>
Link: https://lore.kernel.org/r/20260806020541.2011936-2-kuba@kernel.org
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
 drivers/net/wan/fsl_ucc_hdlc.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 809f21fb93f56..82796452e54a2 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -34,8 +34,6 @@
 #define TDM_PPPOHT_SLIC_MAXIN
 #define RX_BD_ERRORS (R_CD_S | R_OV_S | R_CR_S | R_AB_S | R_NO_S | R_LG_S)
 
-static int uhdlc_close(struct net_device *dev);
-
 static struct ucc_tdm_info utdm_primary_info = {
 	.uf_info = {
 		.tsa = 0,
@@ -705,12 +703,18 @@ static int uhdlc_open(struct net_device *dev)
 	hdlc_device *hdlc = dev_to_hdlc(dev);
 	struct ucc_hdlc_private *priv = hdlc->priv;
 	struct ucc_tdm *utdm = priv->utdm;
-	int rc = 0;
+	int rc;
 
 	if (priv->hdlc_busy != 1) {
+		rc = hdlc_open(dev);
+		if (rc)
+			return rc;
+
 		if (request_irq(priv->ut_info->uf_info.irq,
-				ucc_hdlc_irq_handler, 0, "hdlc", priv))
+				ucc_hdlc_irq_handler, 0, "hdlc", priv)) {
+			hdlc_close(dev);
 			return -ENODEV;
+		}
 
 		cecr_subblock = ucc_fast_get_qe_cr_subblock(
 					priv->ut_info->uf_info.ucc_num);
@@ -729,13 +733,9 @@ static int uhdlc_open(struct net_device *dev)
 		napi_enable(&priv->napi);
 		netdev_reset_queue(dev);
 		netif_start_queue(dev);
-
-		rc = hdlc_open(dev);
-		if (rc)
-			uhdlc_close(dev);
 	}
 
-	return rc;
+	return 0;
 }
 
 static void uhdlc_memclean(struct ucc_hdlc_private *priv)
-- 
2.47.1

[PATCH net v3 2/4] net: wan: fsl_ucc_hdlc: allocate suspend backup before quiescing

[Myeonghun Pak]

uhdlc_suspend() detaches the netdev and disables NAPI before allocating
the parameter RAM backup. If that allocation fails, suspend returns
-ENOMEM with the interface still running but NAPI disabled. The PM core
does not call resume after a failed suspend, so a later close attempts to
disable NAPI again and can wait indefinitely.

Allocate the backup before changing the runtime state. An allocation
failure then leaves the interface attached and NAPI enabled.

Fixes: c19b6d246a35 ("drivers/net: support hdlc function for QE-UCC")
Cc: stable@vger.kernel.org
Reported-by: Jakub Kicinski <kuba@kernel.org>
Link: https://lore.kernel.org/r/20260806020541.2011936-2-kuba@kernel.org
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
 drivers/net/wan/fsl_ucc_hdlc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 82796452e54a2..596f4ef053636 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -888,6 +888,10 @@ static int uhdlc_suspend(struct device *dev)
 	if (!netif_running(priv->ndev))
 		return 0;
 
+	priv->ucc_pram_bak = kmalloc_obj(*priv->ucc_pram_bak);
+	if (!priv->ucc_pram_bak)
+		return -ENOMEM;
+
 	netif_device_detach(priv->ndev);
 	napi_disable(&priv->napi);
 
@@ -897,10 +901,6 @@ static int uhdlc_suspend(struct device *dev)
 	priv->gumr = ioread32be(&uf_regs->gumr);
 	priv->guemr = ioread8(&uf_regs->guemr);
 
-	priv->ucc_pram_bak = kmalloc_obj(*priv->ucc_pram_bak);
-	if (!priv->ucc_pram_bak)
-		return -ENOMEM;
-
 	/* backup HDLC parameter */
 	memcpy_fromio(priv->ucc_pram_bak, priv->ucc_pram,
 		      sizeof(struct ucc_hdlc_param));
-- 
2.47.1

[PATCH net v3 3/4] net: wan: hdlc: close active devices before protocol detach

[Myeonghun Pak]

Commit ff3516442768 ("WAN: HDLC: Detach protocol before unregistering
device") moved protocol detach ahead of netdev unregister so detach could
still use its state. However, detach_hdlc_protocol() calls
hdlc_setup_dev(), which clears IFF_UP. unregister_netdevice() then sees an
already-down device and skips ndo_stop, leaving an active HDLC device
running while its driver releases resources.

Close the device under RTNL while its protocol is still attached, then
keep the existing detach-before-unregister order. This runs the hardware
stop callback and the protocol close callback before their state is
released.

Audit all current users: c101, n2, pc300too, pci200syn, wanxl,
ixp4xx_hss, fsl_qmc_hdlc and farsync unregister before releasing the
resources used by their close callbacks. The farsync probe unwind can
disable interrupts and free its RX DMA buffer first, but fst_close() does
not use that buffer or require interrupts; the card is also in FST_RESET,
so fst_closeport() does not access the port hardware.

Fixes: ff3516442768 ("WAN: HDLC: Detach protocol before unregistering device")
Cc: stable@vger.kernel.org
Reported-by: Jakub Kicinski <kuba@kernel.org>
Link: https://lore.kernel.org/r/20260806020541.2011936-2-kuba@kernel.org
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
 drivers/net/wan/hdlc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wan/hdlc.c b/drivers/net/wan/hdlc.c
index cbed10b1d862e..a38f90d3006ed 100644
--- a/drivers/net/wan/hdlc.c
+++ b/drivers/net/wan/hdlc.c
@@ -263,6 +263,7 @@ EXPORT_SYMBOL(alloc_hdlcdev);
 void unregister_hdlc_device(struct net_device *dev)
 {
 	rtnl_lock();
+	dev_close(dev);
 	detach_hdlc_protocol(dev);
 	unregister_netdevice(dev);
 	rtnl_unlock();
-- 
2.47.1

[PATCH net v3 4/4] net: wan: fsl_ucc_hdlc: release HDLC device on remove

[Myeonghun Pak]

ucc_hdlc_probe() registers an HDLC netdev whose private pointer refers to
the separately allocated ucc_hdlc_private object. The remove path frees
that object and its resources without unregistering or freeing the
netdev. The registered device is left with a dangling private pointer.

Unregister the HDLC device before releasing the UCC and DMA resources so
an active interface is stopped first. Free the netdev before releasing
its private object.

This patch depends on the preceding "net: wan: hdlc: close active devices
before protocol detach" fix (patch 3 of this series). Without that fix,
protocol detach clears IFF_UP before unregister can invoke ndo_stop, so
an active interface would not be stopped before its resources are freed.

Fixes: c19b6d246a35 ("drivers/net: support hdlc function for QE-UCC")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/20260803133048.42650-1-mhun512@gmail.com
Link: https://lore.kernel.org/r/20260806020541.2011936-2-kuba@kernel.org
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
 drivers/net/wan/fsl_ucc_hdlc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 596f4ef053636..371150efc1a65 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -1255,6 +1255,8 @@ static void ucc_hdlc_remove(struct platform_device *pdev)
 {
 	struct ucc_hdlc_private *priv = dev_get_drvdata(&pdev->dev);
 
+	unregister_hdlc_device(priv->ndev);
+
 	uhdlc_memclean(priv);
 
 	if (priv->utdm && priv->utdm->si_regs) {
@@ -1266,6 +1268,7 @@ static void ucc_hdlc_remove(struct platform_device *pdev)
 		iounmap(priv->utdm->siram);
 		priv->utdm->siram = NULL;
 	}
+	free_netdev(priv->ndev);
 	kfree(priv);
 
 	dev_info(&pdev->dev, "UCC based hdlc module removed\n");
-- 
2.47.1