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

  Previously, we were adding synchronous requests to request list, we
were doing it after checking if request existed. However, we only
removed the request from the request list if we have succeeded in
sending the request. In case of failed request send, we left an
invalid pointer in the request list.

Fix this by only adding request to the list once we succeed in
sending it.

Fixes: 783b6e54971d ("eal: add synchronous multi-process communication")
Cc: jianfeng.tan@intel.com

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

On 4/13/2018 7:54 PM, Anatoly Burakov wrote:
> Previously, we were adding synchronous requests to request list, we
> were doing it after checking if request existed. However, we only
> removed the request from the request list if we have succeeded in
> sending the request. In case of failed request send, we left an
> invalid pointer in the request list.
>
> Fix this by only adding request to the list once we succeed in
> sending it.
>
> Fixes: 783b6e54971d ("eal: add synchronous multi-process communication")
> Cc: jianfeng.tan@intel.com
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>

Nice catch.

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


> ---
>   lib/librte_eal/common/eal_common_proc.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c
> index c888c84..e3eb430 100644
> --- a/lib/librte_eal/common/eal_common_proc.c
> +++ b/lib/librte_eal/common/eal_common_proc.c
> @@ -922,8 +922,6 @@ mp_request_sync(const char *dst, struct rte_mp_msg *req,
>   
>   	pthread_mutex_lock(&pending_requests.lock);
>   	exist = find_sync_request(dst, req->name);
> -	if (!exist)
> -		TAILQ_INSERT_TAIL(&pending_requests.requests, &sync_req, next);
>   	if (exist) {
>   		RTE_LOG(ERR, EAL, "A pending request %s:%s\n", dst, req->name);
>   		rte_errno = EEXIST;
> @@ -939,6 +937,8 @@ mp_request_sync(const char *dst, struct rte_mp_msg *req,
>   	} else if (ret == 0)
>   		return 0;
>   
> +	TAILQ_INSERT_TAIL(&pending_requests.requests, &sync_req, next);
> +
>   	reply->nb_sent++;
>   
>   	do {
  

13/04/2018 17:33, Tan, Jianfeng:
> 
> On 4/13/2018 7:54 PM, Anatoly Burakov wrote:
> > Previously, we were adding synchronous requests to request list, we
> > were doing it after checking if request existed. However, we only
> > removed the request from the request list if we have succeeded in
> > sending the request. In case of failed request send, we left an
> > invalid pointer in the request list.
> >
> > Fix this by only adding request to the list once we succeed in
> > sending it.
> >
> > Fixes: 783b6e54971d ("eal: add synchronous multi-process communication")
> > Cc: jianfeng.tan@intel.com
> >
> > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> 
> Nice catch.
> 
> Acked-by: Jianfeng Tan <jianfeng.tan@intel.com>

Series applied, thanks