Re: [PATCH net-next v10 3/5] net: dsa: mxl862xx: add devlink flash_update and info_get
From: Daniel Golle <daniel@makrotopia.org>
Date: 2026-08-13 09:55:32
Also in:
linux-doc, lkml
On Thu, Aug 13, 2026 at 10:22:33AM +0200, Paolo Abeni wrote:
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. Full review at: https://sashiko.dev/ --- Subject: net: dsa: mxl862xx: add devlink flash_update and info_get This commit implements runtime firmware upgrading for the MxL862xx switch using devlink dev flash. It transfers the firmware image over SB PDI, reboots the switch, and defers a device reprobe to re-initialize the driver using a workqueue.quoted
diff --git a/drivers/net/dsa/mxl862xx/mxl862xx-fw.c b/drivers/net/dsa/mxl862xx/mxl862xx-fw.c new file mode 100644 index 0000000000000..9799cf19d31a9 --- /dev/null +++ b/drivers/net/dsa/mxl862xx/mxl862xx-fw.c[ ... ]quoted
+static void mxl862xx_reprobe_work_fn(struct work_struct *work) +{ + struct mxl862xx_reprobe *rp = + container_of(work, struct mxl862xx_reprobe, work.work); + struct device *dev = rp->dev; + bool bound; + + device_lock(dev); + bound = !!dev_get_drvdata(dev); + device_unlock(dev); + + if (!bound) + dev_dbg(dev, "reprobe skipped, device is gone\n"); + else if (device_reprobe(dev))[Severity: High] Is there a race window here that can lead to a use-after-free of priv? If device_shutdown() runs after device_unlock() but before device_reprobe(), it can clear drvdata. When device_reprobe() subsequently runs, it triggers the driver remove() function, which returns early because drvdata is NULL, bypassing dsa_unregister_switch(). However, because priv was allocated with devm_kzalloc() during probe: drivers/net/dsa/mxl862xx/mxl862xx.c:mxl862xx_probe() { priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); ... } the driver core will still run devres_release_all() and free priv. The DSA switch remains registered, and any subsequent access to it via devlink or netlink could dereference the freed priv pointer.quoted
+ dev_err(dev, "reprobe failed; device left unbound\n"); + + put_device(dev); + kfree(rp); + module_put(THIS_MODULE); +}[Severity: High] Can this cause a kernel panic if the module is unloaded concurrently? Calling module_put(THIS_MODULE) at the very end of a work item means the module reference count drops to zero before the function fully returns to the workqueue core. A concurrent rmmod can then proceed and unmap the module text section. If the worker thread is preempted or delayed, it will resume execution from unmapped memory during the function epilogue, leading to a fatal page fault. -- This is an AI-generated review.
Both issues are real, but hard to trigger in practise. See https://lore.kernel.org/all/anpxFdwNxk0XwPjQ@makrotopia.org/ (local) for a proposal fixing this in other existing drivers (mxl862xx re-probe is modelled after iwlwifi which suffers from the same issue) I suggest to merge this series as is, I'll then convert mxl862xx to use device_schedule_reprobe() once it has landed.