* [PATCH 0/4] net/virtio-user: fix coverity issues
@ 2016-06-29 9:05 Jianfeng Tan
2016-06-29 9:05 ` [PATCH 1/4] net/virtio-user: fix return value not checked Jianfeng Tan
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Jianfeng Tan @ 2016-06-29 9:05 UTC (permalink / raw)
To: dev; +Cc: yuanhan.liu, huawei.xie, john.mcnamara, Jianfeng Tan
Patch 1: fix return value not checked, Coverity issue: 127344, 127478
Patch 2: fix string overflow, Coverity issue: 127484
Patch 3: fix resource leaks, Coverity issue: 127482
Patch 4: fix string unterminated, Coverity issue: 127476
Jianfeng Tan (4):
net/virtio-user: fix return value not checked
net/virtio-user: fix string overflow
net/virtio-user: fix resource leaks
net/virtio-user: fix string unterminated
drivers/net/virtio/virtio_user/vhost_user.c | 5 +-
drivers/net/virtio/virtio_user/virtio_user_dev.c | 2 +-
drivers/net/virtio/virtio_user_ethdev.c | 61 ++++++++++++++++++------
3 files changed, 50 insertions(+), 18 deletions(-)
--
2.1.4
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/4] net/virtio-user: fix return value not checked 2016-06-29 9:05 [PATCH 0/4] net/virtio-user: fix coverity issues Jianfeng Tan @ 2016-06-29 9:05 ` Jianfeng Tan 2016-07-01 2:15 ` Yuanhan Liu 2016-06-29 9:05 ` [PATCH 2/4] net/virtio-user: fix string overflow Jianfeng Tan ` (2 subsequent siblings) 3 siblings, 1 reply; 8+ messages in thread From: Jianfeng Tan @ 2016-06-29 9:05 UTC (permalink / raw) To: dev; +Cc: yuanhan.liu, huawei.xie, john.mcnamara, Jianfeng Tan When return values of function calls are not checked, Coverity will report errors like: if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_PATH) == 1) >>> CID 127477: (CHECKED_RETURN) >>> Calling "rte_kvargs_process" without checking return value (as is done elsewhere 25 out of 30 times). rte_kvargs_process(kvlist, VIRTIO_USER_ARG_PATH, &get_string_arg, &path); Coverity issue: 127344, 127478 Fixes: ce2eabdd43ec ("net/virtio-user: add virtual device") Fixes: 6a84c37e3975 ("net/virtio-user: add vhost-user adapter layer") Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com> --- drivers/net/virtio/virtio_user/vhost_user.c | 3 +- drivers/net/virtio/virtio_user_ethdev.c | 57 ++++++++++++++++++++++------- 2 files changed, 45 insertions(+), 15 deletions(-) diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c index a2b0687..a159ece 100644 --- a/drivers/net/virtio/virtio_user/vhost_user.c +++ b/drivers/net/virtio/virtio_user/vhost_user.c @@ -392,7 +392,8 @@ vhost_user_setup(const char *path) } flag = fcntl(fd, F_GETFD); - fcntl(fd, F_SETFD, flag | FD_CLOEXEC); + if (fcntl(fd, F_SETFD, flag | FD_CLOEXEC) < 0) + PMD_DRV_LOG(WARNING, "fcntl failed, %s", strerror(errno)); memset(&un, 0, sizeof(un)); un.sun_family = AF_UNIX; diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c index 5ab2471..8429b2e 100644 --- a/drivers/net/virtio/virtio_user_ethdev.c +++ b/drivers/net/virtio/virtio_user_ethdev.c @@ -343,29 +343,58 @@ virtio_user_pmd_devinit(const char *name, const char *params) } if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_PATH) == 1) - rte_kvargs_process(kvlist, VIRTIO_USER_ARG_PATH, - &get_string_arg, &path); + ret = rte_kvargs_process(kvlist, VIRTIO_USER_ARG_PATH, + &get_string_arg, &path); + if (ret < 0) { + PMD_INIT_LOG(ERR, "error to parse %s", + VIRTIO_USER_ARG_PATH); + goto end; + } else { PMD_INIT_LOG(ERR, "arg %s is mandatory for virtio-user\n", VIRTIO_USER_ARG_QUEUE_SIZE); goto end; } - if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_MAC) == 1) - rte_kvargs_process(kvlist, VIRTIO_USER_ARG_MAC, - &get_string_arg, &mac_addr); + if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_MAC) == 1) { + ret = rte_kvargs_process(kvlist, VIRTIO_USER_ARG_MAC, + &get_string_arg, &mac_addr); + if (ret < 0) { + PMD_INIT_LOG(ERR, "error to parse %s", + VIRTIO_USER_ARG_MAC); + goto end; + } + } - if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_QUEUE_SIZE) == 1) - rte_kvargs_process(kvlist, VIRTIO_USER_ARG_QUEUE_SIZE, - &get_integer_arg, &queue_size); + if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_QUEUE_SIZE) == 1) { + ret = rte_kvargs_process(kvlist, VIRTIO_USER_ARG_QUEUE_SIZE, + &get_integer_arg, &queue_size); + if (ret < 0) { + PMD_INIT_LOG(ERR, "error to parse %s", + VIRTIO_USER_ARG_QUEUE_SIZE); + goto end; + } + } - if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_QUEUES_NUM) == 1) - rte_kvargs_process(kvlist, VIRTIO_USER_ARG_QUEUES_NUM, - &get_integer_arg, &queues); + if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_QUEUES_NUM) == 1) { + ret = rte_kvargs_process(kvlist, VIRTIO_USER_ARG_QUEUES_NUM, + &get_integer_arg, &queues); + if (ret < 0) { + PMD_INIT_LOG(ERR, "error to parse %s", + VIRTIO_USER_ARG_QUEUES_NUM); + goto end; + } + } - if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_CQ_NUM) == 1) - rte_kvargs_process(kvlist, VIRTIO_USER_ARG_CQ_NUM, - &get_integer_arg, &cq); + if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_CQ_NUM) == 1) { + ret = rte_kvargs_process(kvlist, VIRTIO_USER_ARG_CQ_NUM, + &get_integer_arg, &cq); + if (ret < 0) { + PMD_INIT_LOG(ERR, "error to parse %s", + VIRTIO_USER_ARG_CQ_NUM); + goto end; + } + } else if (queues > 1) cq = 1; -- 2.1.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] net/virtio-user: fix return value not checked 2016-06-29 9:05 ` [PATCH 1/4] net/virtio-user: fix return value not checked Jianfeng Tan @ 2016-07-01 2:15 ` Yuanhan Liu 2016-07-05 10:14 ` Tan, Jianfeng 0 siblings, 1 reply; 8+ messages in thread From: Yuanhan Liu @ 2016-07-01 2:15 UTC (permalink / raw) To: Jianfeng Tan; +Cc: dev, huawei.xie, john.mcnamara On Wed, Jun 29, 2016 at 09:05:33AM +0000, Jianfeng Tan wrote: > - if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_CQ_NUM) == 1) > - rte_kvargs_process(kvlist, VIRTIO_USER_ARG_CQ_NUM, > - &get_integer_arg, &cq); > + if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_CQ_NUM) == 1) { > + ret = rte_kvargs_process(kvlist, VIRTIO_USER_ARG_CQ_NUM, > + &get_integer_arg, &cq); > + if (ret < 0) { > + PMD_INIT_LOG(ERR, "error to parse %s", > + VIRTIO_USER_ARG_CQ_NUM); > + goto end; > + } > + } > else if (queues > 1) Above 2 lines should be in one line. Fixed, and series applied to dpdk-next-virtio. Thanks. --yliu ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] net/virtio-user: fix return value not checked 2016-07-01 2:15 ` Yuanhan Liu @ 2016-07-05 10:14 ` Tan, Jianfeng 2016-07-05 11:31 ` Yuanhan Liu 0 siblings, 1 reply; 8+ messages in thread From: Tan, Jianfeng @ 2016-07-05 10:14 UTC (permalink / raw) To: Yuanhan Liu Cc: dev@dpdk.org, Xie, Huawei, Mcnamara, John, Jastrzebski, MichalX K Hi Yuanhan, > -----Original Message----- > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com] > Sent: Friday, July 1, 2016 10:16 AM > To: Tan, Jianfeng > Cc: dev@dpdk.org; Xie, Huawei; Mcnamara, John > Subject: Re: [PATCH 1/4] net/virtio-user: fix return value not checked > > On Wed, Jun 29, 2016 at 09:05:33AM +0000, Jianfeng Tan wrote: > > - if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_CQ_NUM) == 1) > > - rte_kvargs_process(kvlist, VIRTIO_USER_ARG_CQ_NUM, > > - &get_integer_arg, &cq); > > + if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_CQ_NUM) == 1) { > > + ret = rte_kvargs_process(kvlist, > VIRTIO_USER_ARG_CQ_NUM, > > + &get_integer_arg, &cq); > > + if (ret < 0) { > > + PMD_INIT_LOG(ERR, "error to parse %s", > > + VIRTIO_USER_ARG_CQ_NUM); > > + goto end; > > + } > > + } > > else if (queues > 1) > > Above 2 lines should be in one line. Fixed, and series applied to > dpdk-next-virtio. Thanks. But there's another problem in the commit log: the Coverity defect ID should be "127477, 127478" instead of "127344, 127478". Can you please help make it right? Thanks, Jianfeng > > Thanks. > > --yliu ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] net/virtio-user: fix return value not checked 2016-07-05 10:14 ` Tan, Jianfeng @ 2016-07-05 11:31 ` Yuanhan Liu 0 siblings, 0 replies; 8+ messages in thread From: Yuanhan Liu @ 2016-07-05 11:31 UTC (permalink / raw) To: Tan, Jianfeng Cc: dev@dpdk.org, Xie, Huawei, Mcnamara, John, Jastrzebski, MichalX K On Tue, Jul 05, 2016 at 10:14:28AM +0000, Tan, Jianfeng wrote: > > On Wed, Jun 29, 2016 at 09:05:33AM +0000, Jianfeng Tan wrote: > > > - if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_CQ_NUM) == 1) > > > - rte_kvargs_process(kvlist, VIRTIO_USER_ARG_CQ_NUM, > > > - &get_integer_arg, &cq); > > > + if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_CQ_NUM) == 1) { > > > + ret = rte_kvargs_process(kvlist, > > VIRTIO_USER_ARG_CQ_NUM, > > > + &get_integer_arg, &cq); > > > + if (ret < 0) { > > > + PMD_INIT_LOG(ERR, "error to parse %s", > > > + VIRTIO_USER_ARG_CQ_NUM); > > > + goto end; > > > + } > > > + } > > > else if (queues > 1) > > > > Above 2 lines should be in one line. Fixed, and series applied to > > dpdk-next-virtio. > > Thanks. > > But there's another problem in the commit log: the Coverity defect ID should be "127477, 127478" instead of "127344, 127478". Can you please help make it right? Done. --yliu ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/4] net/virtio-user: fix string overflow 2016-06-29 9:05 [PATCH 0/4] net/virtio-user: fix coverity issues Jianfeng Tan 2016-06-29 9:05 ` [PATCH 1/4] net/virtio-user: fix return value not checked Jianfeng Tan @ 2016-06-29 9:05 ` Jianfeng Tan 2016-06-29 9:05 ` [PATCH 3/4] net/virtio-user: fix resource leaks Jianfeng Tan 2016-06-29 9:05 ` [PATCH 4/4] net/virtio-user: fix string unterminated Jianfeng Tan 3 siblings, 0 replies; 8+ messages in thread From: Jianfeng Tan @ 2016-06-29 9:05 UTC (permalink / raw) To: dev; +Cc: yuanhan.liu, huawei.xie, john.mcnamara, Jianfeng Tan When parsing /proc/self/maps to get hugepage information, the string was being copied with strcpy(), which could, theoretically but in fact not possiblly, overflow the destination buffer. Anyway, to avoid the false alarm, we replaced strncpy with snprintf for safely copying the strings. Coverity issue: 127484 Fixes: 6a84c37e3975 ("net/virtio-user: add vhost-user adapter layer") Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com> --- drivers/net/virtio/virtio_user/vhost_user.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c index a159ece..082e821 100644 --- a/drivers/net/virtio/virtio_user/vhost_user.c +++ b/drivers/net/virtio/virtio_user/vhost_user.c @@ -181,7 +181,7 @@ get_hugepage_file_info(struct hugepage_file_info huges[], int max) } huges[idx].addr = v_start; huges[idx].size = v_end - v_start; - strcpy(huges[idx].path, tmp); + snprintf(huges[idx].path, PATH_MAX, "%s", tmp); idx++; } -- 2.1.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/4] net/virtio-user: fix resource leaks 2016-06-29 9:05 [PATCH 0/4] net/virtio-user: fix coverity issues Jianfeng Tan 2016-06-29 9:05 ` [PATCH 1/4] net/virtio-user: fix return value not checked Jianfeng Tan 2016-06-29 9:05 ` [PATCH 2/4] net/virtio-user: fix string overflow Jianfeng Tan @ 2016-06-29 9:05 ` Jianfeng Tan 2016-06-29 9:05 ` [PATCH 4/4] net/virtio-user: fix string unterminated Jianfeng Tan 3 siblings, 0 replies; 8+ messages in thread From: Jianfeng Tan @ 2016-06-29 9:05 UTC (permalink / raw) To: dev; +Cc: yuanhan.liu, huawei.xie, john.mcnamara, Jianfeng Tan The return value by rte_kvargs_parse is not free(d), which leads to memory leak. Coverity issue: 127482 Fixes: ce2eabdd43ec ("net/virtio-user: add virtual device") Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com> --- drivers/net/virtio/virtio_user_ethdev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c index 8429b2e..8e39adf 100644 --- a/drivers/net/virtio/virtio_user_ethdev.c +++ b/drivers/net/virtio/virtio_user_ethdev.c @@ -320,7 +320,7 @@ virtio_user_eth_dev_alloc(const char *name) static int virtio_user_pmd_devinit(const char *name, const char *params) { - struct rte_kvargs *kvlist; + struct rte_kvargs *kvlist = NULL; struct rte_eth_dev *eth_dev; struct virtio_hw *hw; uint64_t queues = VIRTIO_USER_DEF_Q_NUM; @@ -422,6 +422,8 @@ virtio_user_pmd_devinit(const char *name, const char *params) ret = 0; end: + if (kvlist) + rte_kvargs_free(kvlist); if (path) free(path); if (mac_addr) -- 2.1.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/4] net/virtio-user: fix string unterminated 2016-06-29 9:05 [PATCH 0/4] net/virtio-user: fix coverity issues Jianfeng Tan ` (2 preceding siblings ...) 2016-06-29 9:05 ` [PATCH 3/4] net/virtio-user: fix resource leaks Jianfeng Tan @ 2016-06-29 9:05 ` Jianfeng Tan 3 siblings, 0 replies; 8+ messages in thread From: Jianfeng Tan @ 2016-06-29 9:05 UTC (permalink / raw) To: dev; +Cc: yuanhan.liu, huawei.xie, john.mcnamara, Jianfeng Tan When use strcpy() to copy string with length exceeding the last parameter of strcpy(), it may lead to the destination string unterminated. We replaced strncpy with snprintf to make sure it's NULL terminated. Coverity issue: 127476 Fixes: ce2eabdd43ec ("net/virtio-user: add virtual device") Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com> --- drivers/net/virtio/virtio_user/virtio_user_dev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c index 1b1e5bf..376c9cf 100644 --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c @@ -181,7 +181,7 @@ int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, int cq, int queue_size, const char *mac) { - strncpy(dev->path, path, PATH_MAX); + snprintf(dev->path, PATH_MAX, "%s", path); dev->max_queue_pairs = queues; dev->queue_pairs = 1; /* mq disabled by default */ dev->queue_size = queue_size; -- 2.1.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-07-05 11:29 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-06-29 9:05 [PATCH 0/4] net/virtio-user: fix coverity issues Jianfeng Tan 2016-06-29 9:05 ` [PATCH 1/4] net/virtio-user: fix return value not checked Jianfeng Tan 2016-07-01 2:15 ` Yuanhan Liu 2016-07-05 10:14 ` Tan, Jianfeng 2016-07-05 11:31 ` Yuanhan Liu 2016-06-29 9:05 ` [PATCH 2/4] net/virtio-user: fix string overflow Jianfeng Tan 2016-06-29 9:05 ` [PATCH 3/4] net/virtio-user: fix resource leaks Jianfeng Tan 2016-06-29 9:05 ` [PATCH 4/4] net/virtio-user: fix string unterminated Jianfeng Tan
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.