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 18B41A0032; Fri, 18 Feb 2022 10:39:49 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id F254140395; Fri, 18 Feb 2022 10:39:48 +0100 (CET) Received: from mail-il1-f174.google.com (mail-il1-f174.google.com [209.85.166.174]) by mails.dpdk.org (Postfix) with ESMTP id 12B9E40150 for ; Fri, 18 Feb 2022 10:39:47 +0100 (CET) Received: by mail-il1-f174.google.com with SMTP id j5so1497424ila.2 for ; Fri, 18 Feb 2022 01:39:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=rmu6v82ILYQRQQlI1MtR51FtiMXxpfnu5dmBM0X6rhc=; b=lWGX7c0Szux1ebZMxlJWF+vBJveSWbvQOc2BGcOm3BS5XFJ0kPZvNeUHHEI/fcVgsF 2+KxCqxlSOr3dSjoh2RvLTxHoE79DQiCJUBCMqvcdBcxyDjuNsgcfXWIvRdL+fxpvMyt +DJUMeM5r8kHKWH6z/+8eUHbm/QQ5rTefTi15IWO36bwd7biDrAuKk+Ssrbm9WiKy2dB xrlqM+T5hFVhO/HeBXlvOfplKIZgFxNbIpdMV3J5pssFyxhniF7Yrn5urhr2+2wdK2Pk bzDBvaBR7gHJ3QwfYGTVUtV14sUneKAgwDfcllls5ZYB+Oo3TP0MTYxzoeMM6m9z2qdJ 2Rtw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=rmu6v82ILYQRQQlI1MtR51FtiMXxpfnu5dmBM0X6rhc=; b=Qt3OLMe/HLpHA//qM/KPTRMClGM58tCz1GJHC1pRei5aldwK7Qiml+b9p/n+RbA/gS F2gKhfejDpy5Csz/Bvy80o0S6w6aN2bR3vKDAjNa+4zcQCtF+md495gtzCCZeOh8WJRu 6alJBfMJirfvHVVg/3di18EstdEeSFqgajHN+Y+aL5zqdZo+XzS2Vx12ebGYWha5RTPt 32bEBnJ6wxpokEXgfc90Jzc/n4SoL1qMvjS0tM6W+XJ1lwoDeNlbeSgfjBxqos26LmwL DhesNhjPvUuLkBfS21ZOkkpFe5knvuKBdTbK4NbA+6VSgmyz0xTTQZiSEcAVHf94Pw64 I9bg== X-Gm-Message-State: AOAM530gr/Qf8lzrkrdN/3gdYCCRjKiMOEg0708nzxrQjQSITed29Sn9 B16jWLnmwlz9nlAOEkd6mTkNxU+uDkAfGtCIpRgwojT9egfWmg== X-Google-Smtp-Source: ABdhPJzpy71U+rzHbuep3gMvgywh3HqZ6Jy2FM4cmcFdn9i2wmpJxxy6in1knpzNtwOKK4vXin8iSIyGthTRGx6sjQA= X-Received: by 2002:a05:6e02:1788:b0:2be:ffc9:8bb2 with SMTP id y8-20020a056e02178800b002beffc98bb2mr5159948ilu.294.1645177186380; Fri, 18 Feb 2022 01:39:46 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Jerin Jacob Date: Fri, 18 Feb 2022 15:09:20 +0530 Message-ID: Subject: Re: [PATCH v6] eventdev/eth_rx: fix memory leak when token parsing finished To: "Jayatheerthan, Jay" Cc: Weiguo Li , "Kundapura, Ganapati" , "Naga Harish K, S V" , "stephen@networkplumber.org" , "dev@dpdk.org" Content-Type: text/plain; charset="UTF-8" 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 On Fri, Feb 4, 2022 at 3:17 PM Jayatheerthan, Jay wrote: > > Looks good. Thanks Weiguo for posting this patch and addressing all the comments! > > Acked-by: Jay Jayatheerthan < jay.jayatheerthan@intel.com> Applied to dpdk-next-net-eventdev/for-main. Thanks > > -Jay > > > > > -----Original Message----- > > From: Weiguo Li > > Sent: Friday, February 4, 2022 1:51 PM > > To: Jayatheerthan, Jay > > Cc: Kundapura, Ganapati ; Naga Harish K, S V ; > > stephen@networkplumber.org; dev@dpdk.org > > Subject: [PATCH v6] eventdev/eth_rx: fix memory leak when token parsing finished > > > > The memory get from strdup should be freed when parameter parsing > > finished, and also should be freed when error occurs. > > > > Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry") > > Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry") > > > > Signed-off-by: Weiguo Li > > Acked-by: Ganapati Kundapura > > > > --- > > v6: > > * fix double free problem in v5 as Jay noted > > v5: > > * fix according to Jay's comment. > > * fix anothor bypassing the freeing of memory problem > > v4: > > * fix compilation issue > > v3: > > * validate "eth_dev_id" by rte_eth_dev_is_valid_port() > > as Ganapati's comment. > > * validate "token" by macros to reduce code redundancy. > > * use new macros to avoid bypassing the freeing of memory problem. > > v2: > > * add memory check after strdup > > --- > > lib/eventdev/rte_event_eth_rx_adapter.c | 96 ++++++++++++++++++------- > > 1 file changed, 72 insertions(+), 24 deletions(-) > > > > diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c > > index ae1e260c08..4da6f1ff90 100644 > > --- a/lib/eventdev/rte_event_eth_rx_adapter.c > > +++ b/lib/eventdev/rte_event_eth_rx_adapter.c > > @@ -293,6 +293,30 @@ rxa_event_buf_get(struct event_eth_rx_adapter *rx_adapter, uint16_t eth_dev_id, > > } \ > > } while (0) > > > > +#define RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(id, retval) do { \ > > + if (!rxa_validate_id(id)) { \ > > + RTE_EDEV_LOG_ERR("Invalid eth Rx adapter id = %d\n", id); \ > > + ret = retval; \ > > + goto error; \ > > + } \ > > +} while (0) > > + > > +#define RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, retval) do { \ > > + if ((token) == NULL || strlen(token) == 0 || !isdigit(*token)) { \ > > + RTE_EDEV_LOG_ERR("Invalid eth Rx adapter token\n"); \ > > + ret = retval; \ > > + goto error; \ > > + } \ > > +} while (0) > > + > > +#define RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(port_id, retval) do { \ > > + if (!rte_eth_dev_is_valid_port(port_id)) { \ > > + RTE_ETHDEV_LOG(ERR, "Invalid port_id=%u\n", port_id); \ > > + ret = retval; \ > > + goto error; \ > > + } \ > > +} while (0) > > + > > static inline int > > rxa_sw_adapter_queue_count(struct event_eth_rx_adapter *rx_adapter) > > { > > @@ -3323,7 +3347,7 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused, > > { > > uint8_t rx_adapter_id; > > uint16_t rx_queue_id; > > - int eth_dev_id; > > + int eth_dev_id, ret = -1; > > char *token, *l_params; > > struct rte_event_eth_rx_adapter_queue_conf queue_conf; > > > > @@ -3332,33 +3356,37 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused, > > > > /* Get Rx adapter ID from parameter string */ > > l_params = strdup(params); > > + if (l_params == NULL) > > + return -ENOMEM; > > token = strtok(l_params, ","); > > + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1); > > rx_adapter_id = strtoul(token, NULL, 10); > > - RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL); > > + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL); > > > > token = strtok(NULL, ","); > > - if (token == NULL || strlen(token) == 0 || !isdigit(*token)) > > - return -1; > > + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1); > > > > /* Get device ID from parameter string */ > > eth_dev_id = strtoul(token, NULL, 10); > > - RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL); > > + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL); > > > > token = strtok(NULL, ","); > > - if (token == NULL || strlen(token) == 0 || !isdigit(*token)) > > - return -1; > > + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1); > > > > /* Get Rx queue ID from parameter string */ > > rx_queue_id = strtoul(token, NULL, 10); > > if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) { > > RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id); > > - return -EINVAL; > > + ret = -EINVAL; > > + goto error; > > } > > > > token = strtok(NULL, "\0"); > > if (token != NULL) > > RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev" > > " telemetry command, ignoring"); > > + /* Parsing parameter finished */ > > + free(l_params); > > > > if (rte_event_eth_rx_adapter_queue_conf_get(rx_adapter_id, eth_dev_id, > > rx_queue_id, &queue_conf)) { > > @@ -3378,6 +3406,10 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused, > > RXA_ADD_DICT(queue_conf.ev, flow_id); > > > > return 0; > > + > > +error: > > + free(l_params); > > + return ret; > > } > > > > static int > > @@ -3387,7 +3419,7 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused, > > { > > uint8_t rx_adapter_id; > > uint16_t rx_queue_id; > > - int eth_dev_id; > > + int eth_dev_id, ret = -1; > > char *token, *l_params; > > struct rte_event_eth_rx_adapter_queue_stats q_stats; > > > > @@ -3396,33 +3428,37 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused, > > > > /* Get Rx adapter ID from parameter string */ > > l_params = strdup(params); > > + if (l_params == NULL) > > + return -ENOMEM; > > token = strtok(l_params, ","); > > + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1); > > rx_adapter_id = strtoul(token, NULL, 10); > > - RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL); > > + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL); > > > > token = strtok(NULL, ","); > > - if (token == NULL || strlen(token) == 0 || !isdigit(*token)) > > - return -1; > > + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1); > > > > /* Get device ID from parameter string */ > > eth_dev_id = strtoul(token, NULL, 10); > > - RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL); > > + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL); > > > > token = strtok(NULL, ","); > > - if (token == NULL || strlen(token) == 0 || !isdigit(*token)) > > - return -1; > > + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1); > > > > /* Get Rx queue ID from parameter string */ > > rx_queue_id = strtoul(token, NULL, 10); > > if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) { > > RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id); > > - return -EINVAL; > > + ret = -EINVAL; > > + goto error; > > } > > > > token = strtok(NULL, "\0"); > > if (token != NULL) > > RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev" > > " telemetry command, ignoring"); > > + /* Parsing parameter finished */ > > + free(l_params); > > > > if (rte_event_eth_rx_adapter_queue_stats_get(rx_adapter_id, eth_dev_id, > > rx_queue_id, &q_stats)) { > > @@ -3441,6 +3477,10 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused, > > RXA_ADD_DICT(q_stats, rx_dropped); > > > > return 0; > > + > > +error: > > + free(l_params); > > + return ret; > > } > > > > static int > > @@ -3450,7 +3490,7 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused, > > { > > uint8_t rx_adapter_id; > > uint16_t rx_queue_id; > > - int eth_dev_id; > > + int eth_dev_id, ret = -1; > > char *token, *l_params; > > > > if (params == NULL || strlen(params) == 0 || !isdigit(*params)) > > @@ -3458,33 +3498,37 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused, > > > > /* Get Rx adapter ID from parameter string */ > > l_params = strdup(params); > > + if (l_params == NULL) > > + return -ENOMEM; > > token = strtok(l_params, ","); > > + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1); > > rx_adapter_id = strtoul(token, NULL, 10); > > - RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL); > > + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL); > > > > token = strtok(NULL, ","); > > - if (token == NULL || strlen(token) == 0 || !isdigit(*token)) > > - return -1; > > + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1); > > > > /* Get device ID from parameter string */ > > eth_dev_id = strtoul(token, NULL, 10); > > - RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL); > > + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL); > > > > token = strtok(NULL, ","); > > - if (token == NULL || strlen(token) == 0 || !isdigit(*token)) > > - return -1; > > + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1); > > > > /* Get Rx queue ID from parameter string */ > > rx_queue_id = strtoul(token, NULL, 10); > > if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) { > > RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id); > > - return -EINVAL; > > + ret = -EINVAL; > > + goto error; > > } > > > > token = strtok(NULL, "\0"); > > if (token != NULL) > > RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev" > > " telemetry command, ignoring"); > > + /* Parsing parameter finished */ > > + free(l_params); > > > > if (rte_event_eth_rx_adapter_queue_stats_reset(rx_adapter_id, > > eth_dev_id, > > @@ -3494,6 +3538,10 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused, > > } > > > > return 0; > > + > > +error: > > + free(l_params); > > + return ret; > > } > > > > RTE_INIT(rxa_init_telemetry) > > -- > > 2.25.1 >