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 9D350A0C4A; Tue, 13 Jul 2021 12:10:33 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0727E4121C; Tue, 13 Jul 2021 12:10:33 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id CF407410E6; Tue, 13 Jul 2021 12:10:30 +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)) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 75A077F519; Tue, 13 Jul 2021 13:10:30 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 75A077F519 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1626171030; bh=Jp365kY1ORhBJbinU0ZmvgL7KTgQJyOjKyafVIDh24M=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=xdYJavVpTLQQaD5gzrGcr+SuzhJa7DapTmFuA4bBZKC8QYELBTGeuFBPsQbEFEMUn FfJN+yg7gBuO8kJ2gX1ZgozZReQQpAAEa5s3/d3QQ8J5xML0GLYmaLGiR5G5jDaP6+ s+MEtBdbyrgOpH+PNcFoS8KsUg7s2aRaH0LE0lGY= To: dapengx.yu@intel.com, Jasvinder Singh , Cristian Dumitrescu Cc: dev@dpdk.org, stable@dpdk.org References: <20210708084457.690538-1-dapengx.yu@intel.com> <20210713100801.597956-1-dapengx.yu@intel.com> From: Andrew Rybchenko Organization: OKTET Labs Message-ID: <85aa208a-6e06-a06c-408b-0982dd175ef9@oktetlabs.ru> Date: Tue, 13 Jul 2021 13:10:30 +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: <20210713100801.597956-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 v2] 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" On 7/13/21 1:08 PM, 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 > --- > V2: > * improve the patch according to maintainer's comment: > rte_strscpy() is the best option here. > --- > 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..8a49e83dce 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; > + rte_strscpy(p->firmware, SOFTNIC_FIRMWARE, sizeof(p->firmware)); I still don't understand why return value is not checked here and in similar cases below. > 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; > } > + rte_strscpy(p->firmware, firmware, sizeof(p->firmware)); > + 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; > + rte_strscpy(p.name, name, sizeof(p.name)); > > /* 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. */ >