diff mbox series

[06/10] lib/librte_pipeline: fix the use of unsafe strcpy

Message ID 1618839289-33224-7-git-send-email-humin29@huawei.com (mailing list archive)
State Rejected, archived
Delegated to: Thomas Monjalon
Headers
Series fixes for clean code |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

humin (Q) April 19, 2021, 1:34 p.m. UTC 
  From: HongBo Zheng <zhenghongbo3@huawei.com>

'strcpy' is called in rte_swx_ctl_table_info_get, this function
is unsafe, use 'strncpy' instead.

Fixes: 393b96e2aa2a ("pipeline: add SWX pipeline query API")
Cc: stable@dpdk.org

Signed-off-by: HongBo Zheng <zhenghongbo3@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 lib/librte_pipeline/rte_swx_pipeline.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Andrew Rybchenko April 20, 2021, 9:36 a.m. UTC | #1 
On 4/19/21 4:34 PM, Min Hu (Connor) wrote:
> From: HongBo Zheng <zhenghongbo3@huawei.com>
> 
> 'strcpy' is called in rte_swx_ctl_table_info_get, this function
> is unsafe, use 'strncpy' instead.
> 
> Fixes: 393b96e2aa2a ("pipeline: add SWX pipeline query API")
> Cc: stable@dpdk.org
> 
> Signed-off-by: HongBo Zheng <zhenghongbo3@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>  lib/librte_pipeline/rte_swx_pipeline.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_pipeline/rte_swx_pipeline.c b/lib/librte_pipeline/rte_swx_pipeline.c
> index 4455d91..d4db4dd 100644
> --- a/lib/librte_pipeline/rte_swx_pipeline.c
> +++ b/lib/librte_pipeline/rte_swx_pipeline.c
> @@ -9447,8 +9447,8 @@ rte_swx_ctl_table_info_get(struct rte_swx_pipeline *p,
>  	if (!t)
>  		return -EINVAL;
>  
> -	strcpy(table->name, t->name);
> -	strcpy(table->args, t->args);
> +	strncpy(table->name, t->name, RTE_SWX_CTL_NAME_SIZE);
> +	strncpy(table->args, t->args, RTE_SWX_CTL_NAME_SIZE);

strlcpy() should be used in fact, since strncpy() has problems
as well.

>  	table->n_match_fields = t->n_fields;
>  	table->n_actions = t->n_actions;
>  	table->default_action_is_const = t->default_action_is_const;
>
Stephen Hemminger June 30, 2023, 6:08 p.m. UTC | #2 
On Mon, 19 Apr 2021 21:34:45 +0800
"Min Hu (Connor)" <humin29@huawei.com> wrote:

> From: HongBo Zheng <zhenghongbo3@huawei.com>
> 
> 'strcpy' is called in rte_swx_ctl_table_info_get, this function
> is unsafe, use 'strncpy' instead.
> 
> Fixes: 393b96e2aa2a ("pipeline: add SWX pipeline query API")
> Cc: stable@dpdk.org
> 
> Signed-off-by: HongBo Zheng <zhenghongbo3@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>  lib/librte_pipeline/rte_swx_pipeline.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_pipeline/rte_swx_pipeline.c b/lib/librte_pipeline/rte_swx_pipeline.c
> index 4455d91..d4db4dd 100644
> --- a/lib/librte_pipeline/rte_swx_pipeline.c
> +++ b/lib/librte_pipeline/rte_swx_pipeline.c
> @@ -9447,8 +9447,8 @@ rte_swx_ctl_table_info_get(struct rte_swx_pipeline *p,
>  	if (!t)
>  		return -EINVAL;
>  
> -	strcpy(table->name, t->name);
> -	strcpy(table->args, t->args);
> +	strncpy(table->name, t->name, RTE_SWX_CTL_NAME_SIZE);
> +	strncpy(table->args, t->args, RTE_SWX_CTL_NAME_SIZE);
>  	table->n_match_fields = t->n_fields;
>  	table->n_actions = t->n_actions;
>  	table->default_action_is_const = t->default_action_is_const;

This patch is unnecessary.
Both structures declare the same size for the name and args.
Therefore the strcpy is always safe as long as the table structure
is correctly setup with null terminated string. If not there are worse bugs.
Cristian Dumitrescu July 3, 2023, 10:57 a.m. UTC | #3 
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Friday, June 30, 2023 7:09 PM
> To: Min Hu (Connor) <humin29@huawei.com>
> Cc: dev@dpdk.org; ferruh.yigit@intel.com; Dumitrescu, Cristian
> <cristian.dumitrescu@intel.com>; jerinj@marvell.com;
> jianjay.zhou@huawei.com; jia.guo@intel.com; g.singh@nxp.com;
> andrew.rybchenko@oktetlabs.ru; hemant.agrawal@nxp.com; orika@nvidia.com
> Subject: Re: [dpdk-dev] [PATCH 06/10] lib/librte_pipeline: fix the use of unsafe
> strcpy
> 
> On Mon, 19 Apr 2021 21:34:45 +0800
> "Min Hu (Connor)" <humin29@huawei.com> wrote:
> 
> > From: HongBo Zheng <zhenghongbo3@huawei.com>
> >
> > 'strcpy' is called in rte_swx_ctl_table_info_get, this function
> > is unsafe, use 'strncpy' instead.
> >
> > Fixes: 393b96e2aa2a ("pipeline: add SWX pipeline query API")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: HongBo Zheng <zhenghongbo3@huawei.com>
> > Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> > ---
> >  lib/librte_pipeline/rte_swx_pipeline.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/librte_pipeline/rte_swx_pipeline.c
> b/lib/librte_pipeline/rte_swx_pipeline.c
> > index 4455d91..d4db4dd 100644
> > --- a/lib/librte_pipeline/rte_swx_pipeline.c
> > +++ b/lib/librte_pipeline/rte_swx_pipeline.c
> > @@ -9447,8 +9447,8 @@ rte_swx_ctl_table_info_get(struct rte_swx_pipeline
> *p,
> >  	if (!t)
> >  		return -EINVAL;
> >
> > -	strcpy(table->name, t->name);
> > -	strcpy(table->args, t->args);
> > +	strncpy(table->name, t->name, RTE_SWX_CTL_NAME_SIZE);
> > +	strncpy(table->args, t->args, RTE_SWX_CTL_NAME_SIZE);
> >  	table->n_match_fields = t->n_fields;
> >  	table->n_actions = t->n_actions;
> >  	table->default_action_is_const = t->default_action_is_const;
> 
> This patch is unnecessary.
> Both structures declare the same size for the name and args.
> Therefore the strcpy is always safe as long as the table structure
> is correctly setup with null terminated string. If not there are worse bugs.


+1
Agree with Steve, this is not necessary here.
diff mbox series

Patch

diff --git a/lib/librte_pipeline/rte_swx_pipeline.c b/lib/librte_pipeline/rte_swx_pipeline.c
index 4455d91..d4db4dd 100644
--- a/lib/librte_pipeline/rte_swx_pipeline.c
+++ b/lib/librte_pipeline/rte_swx_pipeline.c
@@ -9447,8 +9447,8 @@  rte_swx_ctl_table_info_get(struct rte_swx_pipeline *p,
 	if (!t)
 		return -EINVAL;
 
-	strcpy(table->name, t->name);
-	strcpy(table->args, t->args);
+	strncpy(table->name, t->name, RTE_SWX_CTL_NAME_SIZE);
+	strncpy(table->args, t->args, RTE_SWX_CTL_NAME_SIZE);
 	table->n_match_fields = t->n_fields;
 	table->n_actions = t->n_actions;
 	table->default_action_is_const = t->default_action_is_const;