[1/8] 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 initializing message buffer
to NULL.
Fixes: 783b6e54971d ("eal: add synchronous multi-process communication")
Cc: jianfeng.tan@intel.com
Cc: jia.quo@intel.com
Cc: gi.z.zhang@intel.com
Cc: stable@dpdk.org
Signed-off-by: Herakliusz Lipiec <herakliusz.lipiec@intel.com>
---
lib/librte_eal/common/eal_common_proc.c | 6 +++---
lib/librte_eal/common/include/rte_eal.h | 3 ++-
2 files changed, 5 insertions(+), 4 deletions(-)
Comments
On 17-Apr-19 3:38 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 initializing message buffer
> to NULL.
>
> Fixes: 783b6e54971d ("eal: add synchronous multi-process communication")
> Cc: jianfeng.tan@intel.com
> Cc: jia.quo@intel.com
> Cc: gi.z.zhang@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Herakliusz Lipiec <herakliusz.lipiec@intel.com>
> ---
> lib/librte_eal/common/eal_common_proc.c | 6 +++---
> lib/librte_eal/common/include/rte_eal.h | 3 ++-
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c
> index b46d644b3..abaff5164 100644
> --- a/lib/librte_eal/common/eal_common_proc.c
> +++ b/lib/librte_eal/common/eal_common_proc.c
> @@ -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)
> + return -1;
> +
> if (internal_config.no_shconf) {
> RTE_LOG(DEBUG, EAL, "No shared files mode enabled, IPC is disabled\n");
> return 0;
> diff --git a/lib/librte_eal/common/include/rte_eal.h b/lib/librte_eal/common/include/rte_eal.h
> index 833433229..575f8119e 100644
> --- a/lib/librte_eal/common/include/rte_eal.h
> +++ b/lib/librte_eal/common/include/rte_eal.h
> @@ -309,7 +309,8 @@ rte_mp_sendmsg(struct rte_mp_msg *msg);
> * This function sends a request message to the peer process, and will
> * block until receiving reply message from the peer process.
> *
> - * @note The caller is responsible to free reply->replies.
> + * @note The caller is responsible to free reply->replies (even if the function
> + * returned failure).
> *
> * @param req
> * The req argument contains the customized request message.
>
These patches should've been submitted as one patchset - otherwise it is
very difficult to review it. Please resubmit as a proper patchset.
As far as the code itself goes,
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
On 17-Apr-19 3:38 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 initializing message buffer
> to NULL.
>
> Fixes: 783b6e54971d ("eal: add synchronous multi-process communication")
> Cc: jianfeng.tan@intel.com
> Cc: jia.quo@intel.com
> Cc: gi.z.zhang@intel.com
> Cc: stable@dpdk.org
Also, Qi Zhang's email is wrong. Please don't write anything out
manually, copy-paste is your friend :)
@@ -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)
+ return -1;
+
if (internal_config.no_shconf) {
RTE_LOG(DEBUG, EAL, "No shared files mode enabled, IPC is disabled\n");
return 0;
@@ -309,7 +309,8 @@ rte_mp_sendmsg(struct rte_mp_msg *msg);
* This function sends a request message to the peer process, and will
* block until receiving reply message from the peer process.
*
- * @note The caller is responsible to free reply->replies.
+ * @note The caller is responsible to free reply->replies (even if the function
+ * returned failure).
*
* @param req
* The req argument contains the customized request message.