* [dpdk-dev] [PATCH 1/2] eal/ipc: fix use-after-free in synchronous requests
@ 2018-04-13 11:54 Anatoly Burakov
2018-04-13 11:55 ` [dpdk-dev] [PATCH 2/2] eal/ipc: fix use-after-free in asynchronous requests Anatoly Burakov
2018-04-13 15:33 ` [dpdk-dev] [PATCH 1/2] eal/ipc: fix use-after-free in synchronous requests Tan, Jianfeng
0 siblings, 2 replies; 5+ messages in thread
From: Anatoly Burakov @ 2018-04-13 11:54 UTC (permalink / raw)
To: dev; +Cc: jianfeng.tan
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(-)
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 {
--
2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread* [dpdk-dev] [PATCH 2/2] eal/ipc: fix use-after-free in asynchronous requests
2018-04-13 11:54 [dpdk-dev] [PATCH 1/2] eal/ipc: fix use-after-free in synchronous requests Anatoly Burakov
@ 2018-04-13 11:55 ` Anatoly Burakov
2018-04-13 15:34 ` Tan, Jianfeng
2018-04-13 15:33 ` [dpdk-dev] [PATCH 1/2] eal/ipc: fix use-after-free in synchronous requests Tan, Jianfeng
1 sibling, 1 reply; 5+ messages in thread
From: Anatoly Burakov @ 2018-04-13 11:55 UTC (permalink / raw)
To: dev; +Cc: jianfeng.tan, anatoly.burakov
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(-)
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++;
--
2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [dpdk-dev] [PATCH 2/2] eal/ipc: fix use-after-free in asynchronous requests
2018-04-13 11:55 ` [dpdk-dev] [PATCH 2/2] eal/ipc: fix use-after-free in asynchronous requests Anatoly Burakov
@ 2018-04-13 15:34 ` Tan, Jianfeng
0 siblings, 0 replies; 5+ messages in thread
From: Tan, Jianfeng @ 2018-04-13 15:34 UTC (permalink / raw)
To: Anatoly Burakov, dev
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++;
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] eal/ipc: fix use-after-free in synchronous requests
2018-04-13 11:54 [dpdk-dev] [PATCH 1/2] eal/ipc: fix use-after-free in synchronous requests Anatoly Burakov
2018-04-13 11:55 ` [dpdk-dev] [PATCH 2/2] eal/ipc: fix use-after-free in asynchronous requests Anatoly Burakov
@ 2018-04-13 15:33 ` Tan, Jianfeng
2018-04-16 23:19 ` Thomas Monjalon
1 sibling, 1 reply; 5+ messages in thread
From: Tan, Jianfeng @ 2018-04-13 15:33 UTC (permalink / raw)
To: Anatoly Burakov, dev
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 {
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [dpdk-dev] [PATCH 1/2] eal/ipc: fix use-after-free in synchronous requests
2018-04-13 15:33 ` [dpdk-dev] [PATCH 1/2] eal/ipc: fix use-after-free in synchronous requests Tan, Jianfeng
@ 2018-04-16 23:19 ` Thomas Monjalon
0 siblings, 0 replies; 5+ messages in thread
From: Thomas Monjalon @ 2018-04-16 23:19 UTC (permalink / raw)
To: Anatoly Burakov; +Cc: dev, Tan, Jianfeng
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-04-16 23:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-13 11:54 [dpdk-dev] [PATCH 1/2] eal/ipc: fix use-after-free in synchronous requests Anatoly Burakov
2018-04-13 11:55 ` [dpdk-dev] [PATCH 2/2] eal/ipc: fix use-after-free in asynchronous requests Anatoly Burakov
2018-04-13 15:34 ` Tan, Jianfeng
2018-04-13 15:33 ` [dpdk-dev] [PATCH 1/2] eal/ipc: fix use-after-free in synchronous requests Tan, Jianfeng
2018-04-16 23:19 ` Thomas Monjalon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).