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: [email protected]

Signed-off-by: Anatoly Burakov <[email protected]>
---
 lib/eal/common/eal_common_proc.c | 34 +++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/lib/eal/common/eal_common_proc.c b/lib/eal/common/eal_common_proc.c
index 869ce99bf9..5cd1bb8d13 100644
--- a/lib/eal/common/eal_common_proc.c
+++ b/lib/eal/common/eal_common_proc.c
@@ -1242,7 +1242,34 @@ rte_mp_request_async(struct rte_mp_msg *req, const 
struct timespec *ts,
                } else if (mp_request_async(path, copy, param, ts))
                        ret = -1;
        }
-       /* if we didn't send anything, put dummy request on the queue */
+
+       /*
+        * 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);
+                       }
+               }
+               reply->nb_sent = 0;
+       }
+
+       /* if we didn't send anything, put dummy request on the queue
+        * and set a minimum-delay alarm so the callback fires immediately.
+        */
        if (ret == 0 && reply->nb_sent == 0) {
                TAILQ_INSERT_HEAD(&pending_requests.requests, dummy, next);
                dummy_used = true;
@@ -1260,6 +1287,11 @@ rte_mp_request_async(struct rte_mp_msg *req, const 
struct timespec *ts,
        /* if dummy was unused, free it */
        if (!dummy_used)
                free(dummy);
+       /* if nothing was sent, nobody owns copy/param */
+       if (ret != 0) {
+               free(param);
+               free(copy);
+       }
 
        return ret;
 closedir_fail:
-- 
2.47.3

Reply via email to