Re: [dpdk-stable] patch 'net/softnic: fix memory leak in arguments parsing' has been queued to stable release 19.11.10

[Christian Ehrhardt <christian.ehrhardt@canonical.com>]

On Tue, Aug 10, 2021 at 5:44 PM <christian.ehrhardt@canonical.com> wrote:
>
> Hi,
>
> FYI, your patch has been queued to stable release 19.11.10
>

Hi,
while applying cleanly your patch caused build time failures like:

[  262s]   if (rte_strscpy(p->firmware, SOFTNIC_FIRMWARE,
[  262s]   ^
[  262s] /home/abuild/rpmbuild/BUILD/dpdk-1628675870.0ca0a4945/drivers/net/softnic/rte_eth_softnic.c:451:2:
error: nested extern declaration of 'rte_strscpy'
[-Werror=nested-externs]
[  262s] /home/abuild/rpmbuild/BUILD/dpdk-1628675870.0ca0a4945/drivers/net/softnic/rte_eth_softnic.c:
At top level:
[  262s] cc1: error: unrecognized command line option
"-Wno-address-of-packed-member" [-Werror]

Therefore the patch will be de-qeueud from the stable branch that shall become
19.11.10.
Please consider having a look and providing a backport.

A backport should contain a reference to the DPDK main branch commit
in it's commit message in the following fashion:
    [ upstream commit <commit's dpdk main branch SHA-1 checksum> ]

For example:
    https://git.dpdk.org/dpdk-stable/commit/?h=18.11&id=d90e6ae6f936ecdc2fd3811ff9f26aec7f3c06eb

When sending the backported patch, please indicate the target branch in the
subject line, as we have multiple branches, for example:
    [PATCH 19.11] foo/bar: fix baz

With git format-patch, this can be achieved by appending the parameter:
    --subject-prefix='PATCH 19.11'

Send the backported patch to "stable@dpdk.org" but not "dev@dpdk.org".

FYI, branch 19.11 is located at tree:
   https://git.dpdk.org/dpdk-stable

Thanks in advance,
Chrtistian


> Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
> It will be pushed if I get no objections before 08/12/21. So please
> shout if anyone has objections.
>
> Also note that after the patch there's a diff of the upstream commit vs the
> patch applied to the branch. This will indicate if there was any rebasing
> needed to apply to the stable branch. If there were code changes for rebasing
> (ie: not only metadata diffs), please double check that the rebase was
> correctly done.
>
> Queued patches are on a temporary branch at:
> https://github.com/cpaelzer/dpdk-stable-queue
>
> This queued commit can be viewed at:
> https://github.com/cpaelzer/dpdk-stable-queue/commit/d313979fd603c8c588822a675b328fd000e17734
>
> Thanks.
>
> Christian Ehrhardt <christian.ehrhardt@canonical.com>
>
> ---
> From d313979fd603c8c588822a675b328fd000e17734 Mon Sep 17 00:00:00 2001
> From: Dapeng Yu <dapengx.yu@intel.com>
> Date: Thu, 15 Jul 2021 13:38:14 +0800
> Subject: [PATCH] net/softnic: fix memory leak in arguments parsing
>
> [ upstream commit d8f852f5f3692bbf15743d1aca25e6abcbe652ad ]
>
> In function pmd_parse_args(), firmware path is duplicated from device
> arguments as character string, but is never freed, which cause memory
> leak.
>
> This patch changes the type of firmware member of struct pmd_params to
> character array, to make memory resource release unnecessary, and
> changes the type of name member to character array, to keep the
> consistency of character string handling in struct pmd_params.
>
> Fixes: 7e68bc20f8c8 ("net/softnic: restructure")
>
> Signed-off-by: Dapeng Yu <dapengx.yu@intel.com>
> Acked-by: Jasvinder Singh <jasvinder.singh@intel.com>
> ---
>  drivers/net/softnic/rte_eth_softnic.c         | 30 ++++++++++++++++---
>  .../net/softnic/rte_eth_softnic_internals.h   |  5 ++--
>  2 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/softnic/rte_eth_softnic.c b/drivers/net/softnic/rte_eth_softnic.c
> index 11723778fd..f4858255ce 100644
> --- a/drivers/net/softnic/rte_eth_softnic.c
> +++ b/drivers/net/softnic/rte_eth_softnic.c
> @@ -440,6 +440,7 @@ pmd_parse_args(struct pmd_params *p, const char *params)
>  {
>         struct rte_kvargs *kvlist;
>         int ret = 0;
> +       char *firmware = NULL;
>
>         kvlist = rte_kvargs_parse(params, pmd_valid_args);
>         if (kvlist == NULL)
> @@ -447,7 +448,14 @@ pmd_parse_args(struct pmd_params *p, const char *params)
>
>         /* Set default values */
>         memset(p, 0, sizeof(*p));
> -       p->firmware = SOFTNIC_FIRMWARE;
> +       if (rte_strscpy(p->firmware, SOFTNIC_FIRMWARE,
> +                       sizeof(p->firmware)) < 0) {
> +               PMD_LOG(WARNING,
> +                       "\"%s\": firmware path should be shorter than %zu",
> +                       SOFTNIC_FIRMWARE, sizeof(p->firmware));
> +               ret = -EINVAL;
> +               goto out_free;
> +       }
>         p->cpu_id = SOFTNIC_CPU_ID;
>         p->sc = SOFTNIC_SC;
>         p->tm.n_queues = SOFTNIC_TM_N_QUEUES;
> @@ -468,11 +476,20 @@ pmd_parse_args(struct pmd_params *p, const char *params)
>         /* Firmware script (optional) */
>         if (rte_kvargs_count(kvlist, PMD_PARAM_FIRMWARE) == 1) {
>                 ret = rte_kvargs_process(kvlist, PMD_PARAM_FIRMWARE,
> -                       &get_string, &p->firmware);
> +                       &get_string, &firmware);
>                 if (ret < 0)
>                         goto out_free;
>         }
> -
> +       if (rte_strscpy(p->firmware, firmware,
> +                       sizeof(p->firmware)) < 0) {
> +               PMD_LOG(WARNING,
> +                       "\"%s\": firmware path should be shorter than %zu",
> +                       firmware, sizeof(p->firmware));
> +               free(firmware);
> +               ret = -EINVAL;
> +               goto out_free;
> +       }
> +       free(firmware);
>         /* Connection listening port (optional) */
>         if (rte_kvargs_count(kvlist, PMD_PARAM_CONN_PORT) == 1) {
>                 ret = rte_kvargs_process(kvlist, PMD_PARAM_CONN_PORT,
> @@ -621,7 +638,12 @@ pmd_probe(struct rte_vdev_device *vdev)
>         if (status)
>                 return status;
>
> -       p.name = name;
> +       if (rte_strscpy(p.name, name, sizeof(p.name)) < 0) {
> +               PMD_LOG(WARNING,
> +                       "\"%s\": device name should be shorter than %zu",
> +                       name, sizeof(p.name));
> +               return -EINVAL;
> +       }
>
>         /* Allocate and initialize soft ethdev private data */
>         dev_private = pmd_init(&p);
> diff --git a/drivers/net/softnic/rte_eth_softnic_internals.h b/drivers/net/softnic/rte_eth_softnic_internals.h
> index 6eec43b22b..f8549b2174 100644
> --- a/drivers/net/softnic/rte_eth_softnic_internals.h
> +++ b/drivers/net/softnic/rte_eth_softnic_internals.h
> @@ -28,14 +28,15 @@
>  #include "conn.h"
>
>  #define NAME_SIZE                                            64
> +#define SOFTNIC_PATH_MAX                                     4096
>
>  /**
>   * PMD Parameters
>   */
>
>  struct pmd_params {
> -       const char *name;
> -       const char *firmware;
> +       char name[NAME_SIZE];
> +       char firmware[SOFTNIC_PATH_MAX];
>         uint16_t conn_port;
>         uint32_t cpu_id;
>         int sc; /**< Service cores. */
> --
> 2.32.0
>
> ---
>   Diff of the applied patch vs upstream commit (please double-check if non-empty:
> ---
> --- -   2021-08-10 15:11:16.063474888 +0200
> +++ 0079-net-softnic-fix-memory-leak-in-arguments-parsing.patch 2021-08-10 15:11:13.090638619 +0200
> @@ -1 +1 @@
> -From d8f852f5f3692bbf15743d1aca25e6abcbe652ad Mon Sep 17 00:00:00 2001
> +From d313979fd603c8c588822a675b328fd000e17734 Mon Sep 17 00:00:00 2001
> @@ -5,0 +6,2 @@
> +[ upstream commit d8f852f5f3692bbf15743d1aca25e6abcbe652ad ]
> +
> @@ -16 +17,0 @@
> -Cc: stable@dpdk.org
> @@ -26 +27 @@
> -index f64023256d..0aa7147b13 100644
> +index 11723778fd..f4858255ce 100644
> @@ -91 +92 @@
> -index 1b3186ef0b..07285ca315 100644
> +index 6eec43b22b..f8549b2174 100644



-- 
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd