From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 48DFAA0C48; Tue, 13 Jul 2021 10:44:02 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 250AC411BD; Tue, 13 Jul 2021 10:44:02 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 1F2ED4069E; Tue, 13 Jul 2021 10:44:01 +0200 (CEST) Received: from [192.168.38.17] (aros.oktetlabs.ru [192.168.38.17]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id AC83B7F523; Tue, 13 Jul 2021 11:44:00 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru AC83B7F523 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1626165840; bh=5DzNyffpEznZWDeqvHZ0+SojeUNA8fzUI0drDJd2F04=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=BxPQEs2+wO/qHxDrMqa23Kx5FXzOaGrUIwe+wqwZyTf3+r7PFbzFVI0hVX9R8C+3h +yNsyJS6oEkQQcNmUtW3o2tV5egK6xIA6M6PsJCn35wd/iq/smnDmulgeEp5TW8shu 17gJVG/+mbaZKGfbw/KJJxFWLsot1k28RD+zr+LM= To: Jasvinder Singh , Cristian Dumitrescu , dapengx.yu@intel.com Cc: dev@dpdk.org, stable@dpdk.org References: <20210708084457.690538-1-dapengx.yu@intel.com> From: Andrew Rybchenko Organization: OKTET Labs Message-ID: <14cca625-2d2e-4949-ef8f-a4ac2570c085@oktetlabs.ru> Date: Tue, 13 Jul 2021 11:44:00 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <20210708084457.690538-1-dapengx.yu@intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] net/softnic: fix memory leak in parsing arguments X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" @Jasvinder, @Cristian, could you the patch, please. On 7/8/21 11:44 AM, dapengx.yu@intel.com wrote: > From: Dapeng Yu > > 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") > Cc: stable@dpdk.org > > Signed-off-by: Dapeng Yu > --- > drivers/net/softnic/rte_eth_softnic.c | 9 ++++++--- > drivers/net/softnic/rte_eth_softnic_internals.h | 4 ++-- > 2 files changed, 8 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/softnic/rte_eth_softnic.c b/drivers/net/softnic/rte_eth_softnic.c > index f64023256d..4d2f93e9c6 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,7 @@ pmd_parse_args(struct pmd_params *p, const char *params) > > /* Set default values */ > memset(p, 0, sizeof(*p)); > - p->firmware = SOFTNIC_FIRMWARE; > + snprintf(p->firmware, sizeof(p->firmware), "%s", SOFTNIC_FIRMWARE); Looking at strlcpy(), rte_strlcpy() and rte_strscpy() I think that rte_strscpy() is the best option here since it is easier to check return value to protect against buffer overflow and shortened result. > p->cpu_id = SOFTNIC_CPU_ID; > p->sc = SOFTNIC_SC; > p->tm.n_queues = SOFTNIC_TM_N_QUEUES; > @@ -468,10 +469,12 @@ 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; > } > + snprintf(p->firmware, sizeof(p->firmware), "%s", firmware); Same here. > + free(firmware); > > /* Connection listening port (optional) */ > if (rte_kvargs_count(kvlist, PMD_PARAM_CONN_PORT) == 1) { > @@ -621,7 +624,7 @@ pmd_probe(struct rte_vdev_device *vdev) > if (status) > return status; > > - p.name = name; > + snprintf(p.name, sizeof(p.name), "%s", name); Same here. > > /* 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 1b3186ef0b..a13d67b7c1 100644 > --- a/drivers/net/softnic/rte_eth_softnic_internals.h > +++ b/drivers/net/softnic/rte_eth_softnic_internals.h > @@ -34,8 +34,8 @@ > */ > > struct pmd_params { > - const char *name; > - const char *firmware; > + char name[RTE_DEV_NAME_MAX_LEN]; > + char firmware[PATH_MAX]; > uint16_t conn_port; > uint32_t cpu_id; > int sc; /**< Service cores. */ >