zynqmp_r5_setup_mbox() installs zynqmp_r5_mb_rx_cb() as the mailbox RX
callback before requesting the mailbox channels, but initializes
ipi->mbox_work only after both channels have been requested. Once the RX
channel is active, a notification delivered before the late INIT_WORK()
would make the callback queue an uninitialized work item.

Initialize the work item and its owning R5 core pointer before requesting
channels. Also drain the work before freeing the mailbox state, after the
channels have been released so no new callbacks can queue it.

This issue was found by our static analysis tool and then confirmed by
manual review of the mailbox setup sequence. The callback is published
before the channel requests complete, so the work item and the state used
by the work handler should be ready before the mailbox provider can invoke
it.

A QEMU PoC modeled a mailbox notification delivered after the RX callback
became reachable but before the delayed INIT_WORK(). DEBUG_OBJECTS reported
queueing an uninitialized work item from the zynqmp_r5_setup_mbox() path.

This is sent as an RFC because the practical trigger depends on the ZynqMP
IPI mailbox provider and firmware delivery timing. If the provider cannot
invoke the RX callback until after setup returns, this is a defensive
lifecycle cleanup rather than a reachable race on current systems.

Fixes: 5dfb28c257b7 ("remoteproc: xilinx: Add mailbox channels for rpmsg")
Signed-off-by: Runyu Xiao <[email protected]>
---
 drivers/remoteproc/xlnx_r5_remoteproc.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/remoteproc/xlnx_r5_remoteproc.c 
b/drivers/remoteproc/xlnx_r5_remoteproc.c
index 0b7b173d0d26..1477fc542afb 100644
--- a/drivers/remoteproc/xlnx_r5_remoteproc.c
+++ b/drivers/remoteproc/xlnx_r5_remoteproc.c
@@ -260,8 +260,9 @@ static void zynqmp_r5_mb_rx_cb(struct mbox_client *cl, void 
*msg)
  * Function to setup mailboxes related properties
  * return : NULL if failed else pointer to mbox_info
  */
-static struct mbox_info *zynqmp_r5_setup_mbox(struct device *cdev)
+static struct mbox_info *zynqmp_r5_setup_mbox(struct zynqmp_r5_core *r5_core)
 {
+       struct device *cdev = r5_core->dev;
        struct mbox_client *mbox_cl;
        struct mbox_info *ipi;
 
@@ -269,6 +270,9 @@ static struct mbox_info *zynqmp_r5_setup_mbox(struct device 
*cdev)
        if (!ipi)
                return NULL;
 
+       ipi->r5_core = r5_core;
+       INIT_WORK(&ipi->mbox_work, handle_event_notified);
+
        mbox_cl = &ipi->mbox_cl;
        mbox_cl->rx_callback = zynqmp_r5_mb_rx_cb;
        mbox_cl->tx_block = false;
@@ -295,8 +299,6 @@ static struct mbox_info *zynqmp_r5_setup_mbox(struct device 
*cdev)
                return NULL;
        }
 
-       INIT_WORK(&ipi->mbox_work, handle_event_notified);
-
        return ipi;
 }
 
@@ -315,6 +317,8 @@ static void zynqmp_r5_free_mbox(struct mbox_info *ipi)
                ipi->rx_chan = NULL;
        }
 
+       cancel_work_sync(&ipi->mbox_work);
+
        kfree(ipi);
 }
 
@@ -1388,10 +1392,9 @@ static int zynqmp_r5_cluster_init(struct 
zynqmp_r5_cluster *cluster)
                 * If mailbox nodes are disabled using "status" property then
                 * setting up mailbox channels will fail.
                 */
-               ipi = zynqmp_r5_setup_mbox(&child_pdev->dev);
+               ipi = zynqmp_r5_setup_mbox(r5_cores[i]);
                if (ipi) {
                        r5_cores[i]->ipi = ipi;
-                       ipi->r5_core = r5_cores[i];
                }
 
                /*
-- 
2.34.1


Reply via email to