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