[dpdk-dev,2/5] example_ip_pipeline: avoid strncpy issue

Message ID 1441072746-29174-3-git-send-email-stephen@networkplumber.org (mailing list archive)
State Changes Requested, archived
Headers

Commit Message

Stephen Hemminger Sept. 1, 2015, 1:59 a.m. UTC
  If name is so long that it fills buffer, then string would not
be null terminated.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/ip_pipeline/config_parse_tm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
  

Comments

Cristian Dumitrescu Sept. 9, 2015, 6:22 p.m. UTC | #1
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Tuesday, September 1, 2015 4:59 AM
> To: Dumitrescu, Cristian
> Cc: dev@dpdk.org; Stephen Hemminger
> Subject: [PATCH 2/5] example_ip_pipeline: avoid strncpy issue
> 
> If name is so long that it fills buffer, then string would not
> be null terminated.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  examples/ip_pipeline/config_parse_tm.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/examples/ip_pipeline/config_parse_tm.c
> b/examples/ip_pipeline/config_parse_tm.c
> index 84702b0..4a35715 100644
> --- a/examples/ip_pipeline/config_parse_tm.c
> +++ b/examples/ip_pipeline/config_parse_tm.c
> @@ -354,7 +354,9 @@ tm_cfgfile_load_sched_subport(
>  					profile = atoi(entries[j].value);
>  					strncpy(name,
>  						entries[j].name,
> -						sizeof(name));
> +						CFG_NAME_LEN - 1);
> +					name[CFG_NAME_LEN-1] = '\0';
> +
>  					n_tokens = rte_strsplit(
>  						&name[sizeof("pipe")],
>  						strnlen(name,
> CFG_NAME_LEN),
> --
> 2.1.4

Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
  
Bruce Richardson Sept. 10, 2015, 8:44 a.m. UTC | #2
On Mon, Aug 31, 2015 at 06:59:03PM -0700, Stephen Hemminger wrote:
> If name is so long that it fills buffer, then string would not
> be null terminated.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  examples/ip_pipeline/config_parse_tm.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/examples/ip_pipeline/config_parse_tm.c b/examples/ip_pipeline/config_parse_tm.c
> index 84702b0..4a35715 100644
> --- a/examples/ip_pipeline/config_parse_tm.c
> +++ b/examples/ip_pipeline/config_parse_tm.c
> @@ -354,7 +354,9 @@ tm_cfgfile_load_sched_subport(
>  					profile = atoi(entries[j].value);
>  					strncpy(name,
>  						entries[j].name,
> -						sizeof(name));
> +						CFG_NAME_LEN - 1);
> +					name[CFG_NAME_LEN-1] = '\0';
> +
>  					n_tokens = rte_strsplit(
>  						&name[sizeof("pipe")],
>  						strnlen(name, CFG_NAME_LEN),
> -- 
> 2.1.4
> 
Would using snprintf rather than strncpy be tidier? Would save having to worry
about null termination at all.

/Bruce
  

Patch

diff --git a/examples/ip_pipeline/config_parse_tm.c b/examples/ip_pipeline/config_parse_tm.c
index 84702b0..4a35715 100644
--- a/examples/ip_pipeline/config_parse_tm.c
+++ b/examples/ip_pipeline/config_parse_tm.c
@@ -354,7 +354,9 @@  tm_cfgfile_load_sched_subport(
 					profile = atoi(entries[j].value);
 					strncpy(name,
 						entries[j].name,
-						sizeof(name));
+						CFG_NAME_LEN - 1);
+					name[CFG_NAME_LEN-1] = '\0';
+
 					n_tokens = rte_strsplit(
 						&name[sizeof("pipe")],
 						strnlen(name, CFG_NAME_LEN),