[dpdk-dev,2/2] eal/ipc: fix use-after-free in asynchronous requests

Message ID c7fafc231189d9f9287d38b7239306c1e37623dd.1523620361.git.anatoly.burakov@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Anatoly Burakov April 13, 2018, 11:55 a.m. UTC
  Previously, we were removing request from the list only if we
have succeeded to send it. This resulted in leaving an invalid
pointer in the request list.

Fix this by only adding new requests to the request list if we
have succeeded in sending them.

Fixes: f05e26051c15 ("eal: add IPC asynchronous request")
Cc: anatoly.burakov@intel.com

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/common/eal_common_proc.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
  

Comments

Jianfeng Tan April 13, 2018, 3:34 p.m. UTC | #1
On 4/13/2018 7:55 PM, Anatoly Burakov wrote:
> Previously, we were removing request from the list only if we
> have succeeded to send it. This resulted in leaving an invalid
> pointer in the request list.
>
> Fix this by only adding new requests to the request list if we
> have succeeded in sending them.
>
> Fixes: f05e26051c15 ("eal: add IPC asynchronous request")
> Cc: anatoly.burakov@intel.com
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>

Acked-by: Jianfeng Tan <jianfeng.tan@intel.com>

> ---
>   lib/librte_eal/common/eal_common_proc.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c
> index e3eb430..a8ca7b8 100644
> --- a/lib/librte_eal/common/eal_common_proc.c
> +++ b/lib/librte_eal/common/eal_common_proc.c
> @@ -876,9 +876,7 @@ mp_request_async(const char *dst, struct rte_mp_msg *req,
>   	/* queue already locked by caller */
>   
>   	exist = find_sync_request(dst, req->name);
> -	if (!exist) {
> -		TAILQ_INSERT_TAIL(&pending_requests.requests, sync_req, next);
> -	} else {
> +	if (exist) {
>   		RTE_LOG(ERR, EAL, "A pending request %s:%s\n", dst, req->name);
>   		rte_errno = EEXIST;
>   		ret = -1;
> @@ -895,6 +893,7 @@ mp_request_async(const char *dst, struct rte_mp_msg *req,
>   		ret = 0;
>   		goto fail;
>   	}
> +	TAILQ_INSERT_TAIL(&pending_requests.requests, sync_req, next);
>   
>   	param->user_reply.nb_sent++;
>
  

Patch

diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c
index e3eb430..a8ca7b8 100644
--- a/lib/librte_eal/common/eal_common_proc.c
+++ b/lib/librte_eal/common/eal_common_proc.c
@@ -876,9 +876,7 @@  mp_request_async(const char *dst, struct rte_mp_msg *req,
 	/* queue already locked by caller */
 
 	exist = find_sync_request(dst, req->name);
-	if (!exist) {
-		TAILQ_INSERT_TAIL(&pending_requests.requests, sync_req, next);
-	} else {
+	if (exist) {
 		RTE_LOG(ERR, EAL, "A pending request %s:%s\n", dst, req->name);
 		rte_errno = EEXIST;
 		ret = -1;
@@ -895,6 +893,7 @@  mp_request_async(const char *dst, struct rte_mp_msg *req,
 		ret = 0;
 		goto fail;
 	}
+	TAILQ_INSERT_TAIL(&pending_requests.requests, sync_req, next);
 
 	param->user_reply.nb_sent++;