Thread (53 messages) 53 messages, 5 authors, 7d ago

Re: [PATCH v7 4/6] eal: fix async IPC memory leaks on partial failure

From: David Marchand <hidden>
Date: 2026-07-01 09:47:23

On Fri, 26 Jun 2026 at 12:34, Anatoly Burakov [off-list ref] wrote:
quoted hunk ↗ jump to hunk
When rte_mp_request_async() fails to send requests to all peers,
copy and param can lose ownership and leak.

However, we cannot simply free them unconditionally, as "partial failure"
means some requests were already queued and thus still reference `copy` and
`param`, so freeing them directly on the error path can cause
use-after-free when those requests are later handled by the async timeout.

Fix this by rolling back queued requests from the current batch, and reset
nb_sent to 0. Freeing the requests is now safe even if some requests were
sent, as any responses or timeouts will not find the request ID in the
queue and will safely exit without doing anything.

Coverity issue: 501503

Fixes: f05e26051c15 ("eal: add IPC asynchronous request")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <redacted>
---
 lib/eal/common/eal_common_proc.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)
diff --git a/lib/eal/common/eal_common_proc.c b/lib/eal/common/eal_common_proc.c
index 991bf215a3..0cffc7a127 100644
--- a/lib/eal/common/eal_common_proc.c
+++ b/lib/eal/common/eal_common_proc.c
@@ -1245,6 +1245,32 @@ rte_mp_request_async(struct rte_mp_msg *req, const struct timespec *ts,
                } else if (mp_request_async(path, copy, param, ts))
                        ret = -1;
        }
+
+       /*
+        * On partial failure, roll back all queued requests. We hold the lock
+        * so no one else touches the queue. All requests in this batch share
+        * the same param pointer. Stale alarms will fire and harmlessly find
+        * nothing via ID-based lookup.
+        */
+       if (ret != 0 && reply->nb_sent > 0) {
+               struct pending_request *r, *next;
+
+               for (r = TAILQ_FIRST(&pending_requests.requests);
+                               r != NULL; r = next) {
+                       next = TAILQ_NEXT(r, next);
+                       if (r->type == REQUEST_TYPE_ASYNC &&
+                                       r->async.param == param) {
+                               TAILQ_REMOVE(&pending_requests.requests,
+                                               r, next);
+                               free(r->reply);
+                               /* r->request == copy, freed below after the loop */
+                               free(r);
+                       }
+               }
+               /* requests on the queue were removed so keep things consistent */
+               reply->nb_sent = 0;
+       }
+
Please, don't reimplement the safe macro.

I plan to update this with:
@@ -1252,15 +1252,11 @@ rte_mp_request_async(struct rte_mp_msg *req,
const struct timespec *ts,
         * nothing via ID-based lookup.
         */
        if (ret != 0 && reply->nb_sent > 0) {
-               struct pending_request *r, *next;
-
-               for (r = TAILQ_FIRST(&pending_requests.requests);
-                               r != NULL; r = next) {
-                       next = TAILQ_NEXT(r, next);
-                       if (r->type == REQUEST_TYPE_ASYNC &&
-                                       r->async.param == param) {
-                               TAILQ_REMOVE(&pending_requests.requests,
-                                               r, next);
+               struct pending_request *r, *tmp;
+
+               RTE_TAILQ_FOREACH_SAFE(r, &pending_requests.requests,
next, tmp) {
+                       if (r->type == REQUEST_TYPE_ASYNC &&
r->async.param == param) {
+
TAILQ_REMOVE(&pending_requests.requests, r, next);
                                free(r->reply);
                                /* r->request == copy, freed below
after the loop */
                                free(r);

Objection?
If not, I'll update while applying.


-- 
David Marchand
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help