diff mbox

[dpdk-dev,v1,1/1] app/procinfo: buffer null termination fix.

Message ID 1492787196-9101-1-git-send-email-romanx.korynkevych@intel.com (mailing list archive)
State Rejected, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Roman Korynkevych April 21, 2017, 3:06 p.m. UTC 
  Coverity issue: 143252
Fixes: 2deb6b5246d7706448d070335b329d1acb754cee ("app/procinfo: add collectd format and host id")
Cc: stable@dpdk.org

Signed-off-by: Roman Korynkevych <romanx.korynkevych@intel.com>
---
 app/proc_info/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Thomas Monjalon May 6, 2017, 9:03 a.m. UTC | #1 
21/04/2017 17:06, Roman Korynkevych:
> Coverity issue: 143252
> Fixes: 2deb6b5246d7706448d070335b329d1acb754cee ("app/procinfo: add collectd format and host id")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Roman Korynkevych <romanx.korynkevych@intel.com>
> ---
>  app/proc_info/main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/app/proc_info/main.c b/app/proc_info/main.c
> index 16b27b2..97d0352 100644
> --- a/app/proc_info/main.c
> +++ b/app/proc_info/main.c
> @@ -189,7 +189,7 @@ proc_info_preparse_args(int argc, char **argv)
>  				proc_info_usage(prgname);
>  				return -1;
>  			}
> -			strncpy(host_id, argv[i+1], sizeof(host_id));
> +			strncpy(host_id, argv[i+1], sizeof(host_id)-1);

The full array size should be given to strncpy.
However, the call to gethostname below seems wrong as
it does not use the full size.

Maryam, Reshma,
Please review the procinfo patches.
Bruce Richardson May 7, 2017, 8:18 p.m. UTC | #2 
On Sat, May 06, 2017 at 11:03:47AM +0200, Thomas Monjalon wrote:
> 21/04/2017 17:06, Roman Korynkevych:
> > Coverity issue: 143252
> > Fixes: 2deb6b5246d7706448d070335b329d1acb754cee ("app/procinfo: add collectd format and host id")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Roman Korynkevych <romanx.korynkevych@intel.com>
> > ---
> >  app/proc_info/main.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/app/proc_info/main.c b/app/proc_info/main.c
> > index 16b27b2..97d0352 100644
> > --- a/app/proc_info/main.c
> > +++ b/app/proc_info/main.c
> > @@ -189,7 +189,7 @@ proc_info_preparse_args(int argc, char **argv)
> >  				proc_info_usage(prgname);
> >  				return -1;
> >  			}
> > -			strncpy(host_id, argv[i+1], sizeof(host_id));
> > +			strncpy(host_id, argv[i+1], sizeof(host_id)-1);
> 
> The full array size should be given to strncpy.
> However, the call to gethostname below seems wrong as
> it does not use the full size.
> 
> Maryam, Reshma,
> Please review the procinfo patches.

Strncpy is dangerous and should not be used in DPDK. Ideally, I'd like
to see us start using strlcpy(), but in the meantime the best practice
in DPDK seems to be to use snprintf in place of strcpy/strncpy.

/Bruce
diff mbox

Patch

diff --git a/app/proc_info/main.c b/app/proc_info/main.c
index 16b27b2..97d0352 100644
--- a/app/proc_info/main.c
+++ b/app/proc_info/main.c
@@ -189,7 +189,7 @@  proc_info_preparse_args(int argc, char **argv)
 				proc_info_usage(prgname);
 				return -1;
 			}
-			strncpy(host_id, argv[i+1], sizeof(host_id));
+			strncpy(host_id, argv[i+1], sizeof(host_id)-1);
 		}
 	}