* [PATCH 0/3] Coverity fixes for DPDK IPC
@ 2018-04-17 15:46 Anatoly Burakov
2018-04-17 15:46 ` [PATCH 1/3] ipc: use strlcpy where applicable Anatoly Burakov
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Anatoly Burakov @ 2018-04-17 15:46 UTC (permalink / raw)
To: dev; +Cc: thomas
This patchset fixes a few Coverity issues introduced
when various parts of DPDK IPC were added, and explains
away other reported issues.
Coverity issues fixed:
- 272595 - return without mutex unlock
- 272609 - fd leak
Coverity issues intentionally not fixed:
- 260407 - strcpy into fixed size buffer
- Both src and dst strings are fixed size, so this is false
positive
- Hopefully will be silenced by replacing strcpy with strlcpy
- 272565 - strcpy into fixed size buffer
- Same as above
- 272582 - strcpy into fixed size buffer
- Same as above
- 268321 - tainted string
- Not an issue, we handle errors correctly
- 272593 - tainted string
- Same as above
- 272604 - tainted string
- Same as above
- 260410 - not checking return value of rte_thread_setname
- We intentionally don't care if it fails
- 272583 - return without mutex unlock
- Independently discovered and fixed [1]
[1] http://dpdk.org/dev/patchwork/patch/38042/
Anatoly Burakov (3):
ipc: use strlcpy where applicable
ipc: fix return without mutex unlock
ipc: fix resource leak
lib/librte_eal/common/eal_common_proc.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/3] ipc: use strlcpy where applicable 2018-04-17 15:46 [PATCH 0/3] Coverity fixes for DPDK IPC Anatoly Burakov @ 2018-04-17 15:46 ` Anatoly Burakov 2018-04-17 15:46 ` [PATCH 2/3] ipc: fix return without mutex unlock Anatoly Burakov ` (2 subsequent siblings) 3 siblings, 0 replies; 6+ messages in thread From: Anatoly Burakov @ 2018-04-17 15:46 UTC (permalink / raw) To: dev; +Cc: thomas, anatoly.burakov, jianfeng.tan This also silences (or should silence) a few Coverity false positives where we used strcpy before (Coverity complained about not checking buffer size, but source buffers were always known to be sized correctly). Coverity issue: 260407 Coverity issue: 272565 Coverity issue: 272582 Fixes: bacaa2754017 ("eal: add channel for multi-process communication") Fixes: f05e26051c15 ("eal: add IPC asynchronous request") Fixes: 783b6e54971d ("eal: add synchronous multi-process communication") Cc: anatoly.burakov@intel.com Cc: jianfeng.tan@intel.com Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com> --- lib/librte_eal/common/eal_common_proc.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c index 2179f3d..74bc300 100644 --- a/lib/librte_eal/common/eal_common_proc.c +++ b/lib/librte_eal/common/eal_common_proc.c @@ -129,7 +129,7 @@ create_socket_path(const char *name, char *buf, int len) if (strlen(name) > 0) snprintf(buf, len, "%s_%s", prefix, name); else - snprintf(buf, len, "%s", prefix); + strlcpy(buf, prefix, len); } int @@ -200,7 +200,7 @@ rte_mp_action_register(const char *name, rte_mp_t action) rte_errno = ENOMEM; return -1; } - strcpy(entry->action_name, name); + strlcpy(entry->action_name, name, sizeof(entry->action_name)); entry->action = action; pthread_mutex_lock(&mp_mutex_action); @@ -323,8 +323,7 @@ process_msg(struct mp_msg_internal *m, struct sockaddr_un *s) */ struct rte_mp_msg dummy; memset(&dummy, 0, sizeof(dummy)); - snprintf(dummy.name, sizeof(dummy.name), - "%s", msg->name); + strlcpy(dummy.name, msg->name, sizeof(dummy.name)); mp_send(&dummy, s->sun_path, MP_IGN); } else { RTE_LOG(ERR, EAL, "Cannot find action: %s\n", @@ -621,11 +620,11 @@ rte_mp_channel_init(void) /* create filter path */ create_socket_path("*", path, sizeof(path)); - snprintf(mp_filter, sizeof(mp_filter), "%s", basename(path)); + strlcpy(mp_filter, basename(path), sizeof(mp_filter)); /* path may have been modified, so recreate it */ create_socket_path("*", path, sizeof(path)); - snprintf(mp_dir_path, sizeof(mp_dir_path), "%s", dirname(path)); + strlcpy(mp_dir_path, dirname(path), sizeof(mp_dir_path)); /* lock the directory */ dir_fd = open(mp_dir_path, O_RDONLY); @@ -673,11 +672,11 @@ rte_mp_channel_init(void) } /* try best to set thread name */ - snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN, "rte_mp_handle"); + strlcpy(thread_name, "rte_mp_handle", RTE_MAX_THREAD_NAME_LEN); rte_thread_setname(mp_handle_tid, thread_name); /* try best to set thread name */ - snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN, "rte_mp_async_handle"); + strlcpy(thread_name, "rte_mp_async_handle", RTE_MAX_THREAD_NAME_LEN); rte_thread_setname(async_reply_handle_tid, thread_name); /* unlock the directory */ @@ -710,7 +709,7 @@ send_msg(const char *dst_path, struct rte_mp_msg *msg, int type) memset(&dst, 0, sizeof(dst)); dst.sun_family = AF_UNIX; - snprintf(dst.sun_path, sizeof(dst.sun_path), "%s", dst_path); + strlcpy(dst.sun_path, dst_path, sizeof(dst.sun_path)); memset(&msgh, 0, sizeof(msgh)); memset(control, 0, sizeof(control)); @@ -870,7 +869,7 @@ mp_request_async(const char *dst, struct rte_mp_msg *req, memset(reply_msg, 0, sizeof(*reply_msg)); sync_req->type = REQUEST_TYPE_ASYNC; - strcpy(sync_req->dst, dst); + strlcpy(sync_req->dst, dst, sizeof(sync_req->dst)); sync_req->request = req; sync_req->reply = reply_msg; sync_req->async.param = param; @@ -916,7 +915,7 @@ mp_request_sync(const char *dst, struct rte_mp_msg *req, sync_req.type = REQUEST_TYPE_SYNC; sync_req.reply_received = 0; - strcpy(sync_req.dst, dst); + strlcpy(sync_req.dst, dst, sizeof(sync_req.dst)); sync_req.request = req; sync_req.reply = &msg; pthread_cond_init(&sync_req.sync.cond, NULL); -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] ipc: fix return without mutex unlock 2018-04-17 15:46 [PATCH 0/3] Coverity fixes for DPDK IPC Anatoly Burakov 2018-04-17 15:46 ` [PATCH 1/3] ipc: use strlcpy where applicable Anatoly Burakov @ 2018-04-17 15:46 ` Anatoly Burakov 2018-04-17 15:46 ` [PATCH 3/3] ipc: fix resource leak Anatoly Burakov 2018-04-20 14:40 ` [PATCH 0/3] Coverity fixes for DPDK IPC Tan, Jianfeng 3 siblings, 0 replies; 6+ messages in thread From: Anatoly Burakov @ 2018-04-17 15:46 UTC (permalink / raw) To: dev; +Cc: thomas, anatoly.burakov gettimeofday() returning a negative value is highly unlikely, but if it ever happens, we will exit without unlocking the mutex. Arguably at that point we'll have bigger problems, but fix this issue anyway. Coverity issue: 272595 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 | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c index 74bc300..ad02c00 100644 --- a/lib/librte_eal/common/eal_common_proc.c +++ b/lib/librte_eal/common/eal_common_proc.c @@ -525,6 +525,7 @@ async_reply_handle(void *arg __rte_unused) wait_for_async_messages(); if (gettimeofday(&now, NULL) < 0) { + pthread_mutex_unlock(&pending_requests.lock); RTE_LOG(ERR, EAL, "Cannot get current time\n"); break; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] ipc: fix resource leak 2018-04-17 15:46 [PATCH 0/3] Coverity fixes for DPDK IPC Anatoly Burakov 2018-04-17 15:46 ` [PATCH 1/3] ipc: use strlcpy where applicable Anatoly Burakov 2018-04-17 15:46 ` [PATCH 2/3] ipc: fix return without mutex unlock Anatoly Burakov @ 2018-04-17 15:46 ` Anatoly Burakov 2018-04-20 14:40 ` [PATCH 0/3] Coverity fixes for DPDK IPC Tan, Jianfeng 3 siblings, 0 replies; 6+ messages in thread From: Anatoly Burakov @ 2018-04-17 15:46 UTC (permalink / raw) To: dev; +Cc: thomas, anatoly.burakov Coverity issue: 272609 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 | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c index ad02c00..a329708 100644 --- a/lib/librte_eal/common/eal_common_proc.c +++ b/lib/librte_eal/common/eal_common_proc.c @@ -658,6 +658,7 @@ rte_mp_channel_init(void) RTE_LOG(ERR, EAL, "failed to create mp thead: %s\n", strerror(errno)); close(mp_fd); + close(dir_fd); mp_fd = -1; return -1; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 0/3] Coverity fixes for DPDK IPC 2018-04-17 15:46 [PATCH 0/3] Coverity fixes for DPDK IPC Anatoly Burakov ` (2 preceding siblings ...) 2018-04-17 15:46 ` [PATCH 3/3] ipc: fix resource leak Anatoly Burakov @ 2018-04-20 14:40 ` Tan, Jianfeng 2018-04-23 20:34 ` Thomas Monjalon 3 siblings, 1 reply; 6+ messages in thread From: Tan, Jianfeng @ 2018-04-20 14:40 UTC (permalink / raw) To: Anatoly Burakov, dev; +Cc: thomas On 4/17/2018 11:46 PM, Anatoly Burakov wrote: > This patchset fixes a few Coverity issues introduced > when various parts of DPDK IPC were added, and explains > away other reported issues. > > Coverity issues fixed: > - 272595 - return without mutex unlock > - 272609 - fd leak > > Coverity issues intentionally not fixed: > - 260407 - strcpy into fixed size buffer > - Both src and dst strings are fixed size, so this is false > positive > - Hopefully will be silenced by replacing strcpy with strlcpy > - 272565 - strcpy into fixed size buffer > - Same as above > - 272582 - strcpy into fixed size buffer > - Same as above > - 268321 - tainted string > - Not an issue, we handle errors correctly > - 272593 - tainted string > - Same as above > - 272604 - tainted string > - Same as above > - 260410 - not checking return value of rte_thread_setname > - We intentionally don't care if it fails > - 272583 - return without mutex unlock > - Independently discovered and fixed [1] > > [1] http://dpdk.org/dev/patchwork/patch/38042/ > > Anatoly Burakov (3): > ipc: use strlcpy where applicable > ipc: fix return without mutex unlock > ipc: fix resource leak > > lib/librte_eal/common/eal_common_proc.c | 23 ++++++++++++----------- > 1 file changed, 12 insertions(+), 11 deletions(-) > For this series, Acked-by: Jianfeng Tan <jianfeng.tan@intel.com> Thanks for fixing these issues. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/3] Coverity fixes for DPDK IPC 2018-04-20 14:40 ` [PATCH 0/3] Coverity fixes for DPDK IPC Tan, Jianfeng @ 2018-04-23 20:34 ` Thomas Monjalon 0 siblings, 0 replies; 6+ messages in thread From: Thomas Monjalon @ 2018-04-23 20:34 UTC (permalink / raw) To: Anatoly Burakov; +Cc: dev, Tan, Jianfeng > > Anatoly Burakov (3): > > ipc: use strlcpy where applicable > > ipc: fix return without mutex unlock > > ipc: fix resource leak > > > > lib/librte_eal/common/eal_common_proc.c | 23 ++++++++++++----------- > > 1 file changed, 12 insertions(+), 11 deletions(-) > > > > For this series, > > Acked-by: Jianfeng Tan <jianfeng.tan@intel.com> Applied, thanks ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-04-23 20:34 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-17 15:46 [PATCH 0/3] Coverity fixes for DPDK IPC Anatoly Burakov 2018-04-17 15:46 ` [PATCH 1/3] ipc: use strlcpy where applicable Anatoly Burakov 2018-04-17 15:46 ` [PATCH 2/3] ipc: fix return without mutex unlock Anatoly Burakov 2018-04-17 15:46 ` [PATCH 3/3] ipc: fix resource leak Anatoly Burakov 2018-04-20 14:40 ` [PATCH 0/3] Coverity fixes for DPDK IPC Tan, Jianfeng 2018-04-23 20:34 ` Thomas Monjalon
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.