[06/10] lib/librte_pipeline: fix the use of unsafe strcpy
Checks
Commit Message
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
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;
>
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.
> -----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.
@@ -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;