[v1] examples/vhost: fix string split error handling issue

Message ID 20201106032343.9099-1-Cheng1.jiang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series [v1] examples/vhost: fix string split error handling issue |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/travis-robot success Travis build: passed
ci/Intel-compilation success Compilation OK
ci/iol-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Jiang, Cheng1 Nov. 6, 2020, 3:23 a.m. UTC
  Add checking return value of string split function to fix the
coverity issue.

Fixes: 3a04ecb21420 ("examples/vhost: add async vhost args parsing")
Coverity issue: 363739

Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com>
---
 examples/vhost/ioat.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
  

Comments

Maxime Coquelin Nov. 10, 2020, 9:34 a.m. UTC | #1
On 11/6/20 4:23 AM, Cheng Jiang wrote:
> Add checking return value of string split function to fix the
> coverity issue.
> 
> Fixes: 3a04ecb21420 ("examples/vhost: add async vhost args parsing")
> Coverity issue: 363739
> 
> Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com>
> ---
>  examples/vhost/ioat.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/examples/vhost/ioat.c b/examples/vhost/ioat.c
> index b2c74f6537..04806c02d7 100644
> --- a/examples/vhost/ioat.c
> +++ b/examples/vhost/ioat.c
> @@ -54,9 +54,14 @@ open_ioat(const char *value)
>  	}
>  	args_nr = rte_strsplit(substr, strlen(substr),
>  			dma_arg, MAX_VHOST_DEVICE, ',');

I think you should check args_nr > 0 explicitly, and maybe even args_nr
>= 0. And then propagate the error if condition is met.


> -	do {
> +	while (i < args_nr) {
>  		char *arg_temp = dma_arg[i];
> -		rte_strsplit(arg_temp, strlen(arg_temp), ptrs, 2, '@');
> +		uint8_t sub_nr;
> +		sub_nr = rte_strsplit(arg_temp, strlen(arg_temp), ptrs, 2, '@');
> +		if (sub_nr != 2) {
> +			ret = -1;
> +			goto out;
> +		}
>  
>  		start = strstr(ptrs[0], "txd");
>  		if (start == NULL) {
> @@ -105,7 +110,7 @@ open_ioat(const char *value)
>  
>  		dma_info->nr++;
>  		i++;
> -	} while (i < args_nr);
> +	}
>  out:
>  	free(input);
>  	return ret;
>
  
Jiang, Cheng1 Nov. 11, 2020, 12:58 a.m. UTC | #2
Hi,

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Tuesday, November 10, 2020 5:35 PM
> To: Jiang, Cheng1 <cheng1.jiang@intel.com>; Xia, Chenbo
> <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; Fu, Patrick <patrick.fu@intel.com>; Yang, YvonneX
> <yvonnex.yang@intel.com>
> Subject: Re: [PATCH v1] examples/vhost: fix string split error handling issue
> 
> 
> 
> On 11/6/20 4:23 AM, Cheng Jiang wrote:
> > Add checking return value of string split function to fix the coverity
> > issue.
> >
> > Fixes: 3a04ecb21420 ("examples/vhost: add async vhost args parsing")
> > Coverity issue: 363739
> >
> > Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com>
> > ---
> >  examples/vhost/ioat.c | 11 ++++++++---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/examples/vhost/ioat.c b/examples/vhost/ioat.c index
> > b2c74f6537..04806c02d7 100644
> > --- a/examples/vhost/ioat.c
> > +++ b/examples/vhost/ioat.c
> > @@ -54,9 +54,14 @@ open_ioat(const char *value)
> >  	}
> >  	args_nr = rte_strsplit(substr, strlen(substr),
> >  			dma_arg, MAX_VHOST_DEVICE, ',');
> 
> I think you should check args_nr > 0 explicitly, and maybe even args_nr
> >= 0. And then propagate the error if condition is met.
> 

Sure, I'll fix it in the next version.

Thanks,
Cheng

> 
> > -	do {
> > +	while (i < args_nr) {
> >  		char *arg_temp = dma_arg[i];
> > -		rte_strsplit(arg_temp, strlen(arg_temp), ptrs, 2, '@');
> > +		uint8_t sub_nr;
> > +		sub_nr = rte_strsplit(arg_temp, strlen(arg_temp), ptrs, 2,
> '@');
> > +		if (sub_nr != 2) {
> > +			ret = -1;
> > +			goto out;
> > +		}
> >
> >  		start = strstr(ptrs[0], "txd");
> >  		if (start == NULL) {
> > @@ -105,7 +110,7 @@ open_ioat(const char *value)
> >
> >  		dma_info->nr++;
> >  		i++;
> > -	} while (i < args_nr);
> > +	}
> >  out:
> >  	free(input);
> >  	return ret;
> >
  

Patch

diff --git a/examples/vhost/ioat.c b/examples/vhost/ioat.c
index b2c74f6537..04806c02d7 100644
--- a/examples/vhost/ioat.c
+++ b/examples/vhost/ioat.c
@@ -54,9 +54,14 @@  open_ioat(const char *value)
 	}
 	args_nr = rte_strsplit(substr, strlen(substr),
 			dma_arg, MAX_VHOST_DEVICE, ',');
-	do {
+	while (i < args_nr) {
 		char *arg_temp = dma_arg[i];
-		rte_strsplit(arg_temp, strlen(arg_temp), ptrs, 2, '@');
+		uint8_t sub_nr;
+		sub_nr = rte_strsplit(arg_temp, strlen(arg_temp), ptrs, 2, '@');
+		if (sub_nr != 2) {
+			ret = -1;
+			goto out;
+		}
 
 		start = strstr(ptrs[0], "txd");
 		if (start == NULL) {
@@ -105,7 +110,7 @@  open_ioat(const char *value)
 
 		dma_info->nr++;
 		i++;
-	} while (i < args_nr);
+	}
 out:
 	free(input);
 	return ret;