[v3,1/2] ipc: fix rte_mp_request_sync memleak
Checks
Commit Message
When sending multiple requests, rte_mp_request_sync
can succeed sending a few of those requests, but then
fail on a later one and in the end return with rc=-1.
The upper layers - e.g. device hotplug - currently
handles this case as if no messages were sent and no
memory for response buffers was allocated, which is
not true. Fixed by always freeing memory buffers on
failure.
Bugzilla ID: 228
Fixes: 783b6e54971d ("eal: add synchronous multi-process communication")
Cc: jianfeng.tan@intel.com
Cc: stable@dpdk.org
Signed-off-by: Herakliusz Lipiec <herakliusz.lipiec@intel.com>
---
lib/librte_eal/common/eal_common_proc.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
Comments
On 25-Apr-19 12:43 PM, Herakliusz Lipiec wrote:
> When sending multiple requests, rte_mp_request_sync
> can succeed sending a few of those requests, but then
> fail on a later one and in the end return with rc=-1.
> The upper layers - e.g. device hotplug - currently
> handles this case as if no messages were sent and no
> memory for response buffers was allocated, which is
> not true. Fixed by always freeing memory buffers on
> failure.
>
> Bugzilla ID: 228
> Fixes: 783b6e54971d ("eal: add synchronous multi-process communication")
> Cc: jianfeng.tan@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Herakliusz Lipiec <herakliusz.lipiec@intel.com>
> ---
Reviewed-by: Anatoly Burakov <anatoly.burakov@intel.com>
@@ -927,13 +927,13 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
RTE_LOG(DEBUG, EAL, "request: %s\n", req->name);
- if (check_input(req) == false)
- return -1;
-
reply->nb_sent = 0;
reply->nb_received = 0;
reply->msgs = NULL;
+ if (check_input(req) == false)
+ goto err;
+
if (internal_config.no_shconf) {
RTE_LOG(DEBUG, EAL, "No shared files mode enabled, IPC is disabled\n");
return 0;
@@ -942,7 +942,7 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
if (gettimeofday(&now, NULL) < 0) {
RTE_LOG(ERR, EAL, "Failed to get current time\n");
rte_errno = errno;
- return -1;
+ goto err;
}
end.tv_nsec = (now.tv_usec * 1000 + ts->tv_nsec) % 1000000000;
@@ -954,6 +954,8 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
pthread_mutex_lock(&pending_requests.lock);
ret = mp_request_sync(eal_mp_socket_path(), req, reply, &end);
pthread_mutex_unlock(&pending_requests.lock);
+ if (ret)
+ goto err;
return ret;
}
@@ -962,7 +964,7 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
if (!mp_dir) {
RTE_LOG(ERR, EAL, "Unable to open directory %s\n", mp_dir_path);
rte_errno = errno;
- return -1;
+ goto err;
}
dir_fd = dirfd(mp_dir);
@@ -972,7 +974,7 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
mp_dir_path);
closedir(mp_dir);
rte_errno = errno;
- return -1;
+ goto err;
}
pthread_mutex_lock(&pending_requests.lock);
@@ -989,7 +991,7 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
* locks on receive
*/
if (mp_request_sync(path, req, reply, &end))
- ret = -1;
+ goto err;
}
pthread_mutex_unlock(&pending_requests.lock);
/* unlock the directory */
@@ -998,6 +1000,12 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
/* dir_fd automatically closed on closedir */
closedir(mp_dir);
return ret;
+
+err:
+ free(reply->msgs);
+ reply->nb_received = 0;
+ reply->msgs = NULL;
+ return -1;
}
int __rte_experimental