From patchwork Wed May 27 08:37:52 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Baum X-Patchwork-Id: 70598 X-Patchwork-Delegate: rasland@nvidia.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id E26A0A04A4; Wed, 27 May 2020 10:38:49 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9B1CF1D660; Wed, 27 May 2020 10:38:48 +0200 (CEST) Received: from mellanox.co.il (mail-il-dmz.mellanox.com [193.47.165.129]) by dpdk.org (Postfix) with ESMTP id EAF941D54F for ; Wed, 27 May 2020 10:38:46 +0200 (CEST) Received: from Internal Mail-Server by MTLPINE2 (envelope-from matan@mellanox.com) with ESMTPS (AES256-SHA encrypted); 27 May 2020 11:38:42 +0300 Received: from pegasus07.mtr.labs.mlnx (pegasus07.mtr.labs.mlnx [10.210.16.112]) by labmailer.mlnx (8.13.8/8.13.8) with ESMTP id 04R8cgss005678; Wed, 27 May 2020 11:38:42 +0300 From: Michael Baum To: dev@dpdk.org Cc: matan@mellanox.com, viacheslavo@mellanox.com, stable@dpdk.org Date: Wed, 27 May 2020 08:37:52 +0000 Message-Id: <1590568677-11662-1-git-send-email-michaelba@mellanox.com> X-Mailer: git-send-email 1.8.3.1 Subject: [dpdk-dev] [PATCH 1/6] net/mlx5: fix hairpin Tx queue creation error flow X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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" The mlx5_txq_obj_hairpin_new function defines a pointer named tmpl and allocates memory for it using the rte_zmalloc_socket function. Later, this function allocates memory to a variable inside tmpl using the mlx5_devx_cmd_create_sq function. In both cases, if the allocation fails, the code jumps to the error label and frees allocated resources. However, in the first jump there are still no resources to free and the jump only for the line return NULL is unnecessary. Even worse, when it jumps to error label with invalid tmpl it actually does dereference to a null pointer. In contrast, the second jump needs to free the tmpl variable but the function instead of freeing, tries to free the variable that it just failed to allocate, and another variable that has never been allocated. In addition, for another error, the function returns NULL without freeing the tmpl variable before, causing a memory leak. Delete the error label and replace each jump with local return NULL and free tmpl variable if needed. Fixes: ae18a1ae9692 ("net/mlx5: support Tx hairpin queues") Cc: stable@dpdk.org Signed-off-by: Michael Baum Acked-by: Matan Azrad --- drivers/net/mlx5/mlx5_txq.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/drivers/net/mlx5/mlx5_txq.c b/drivers/net/mlx5/mlx5_txq.c index a211fa9..7cc620a 100644 --- a/drivers/net/mlx5/mlx5_txq.c +++ b/drivers/net/mlx5/mlx5_txq.c @@ -493,7 +493,6 @@ container_of(txq_data, struct mlx5_txq_ctrl, txq); struct mlx5_devx_create_sq_attr attr = { 0 }; struct mlx5_txq_obj *tmpl = NULL; - int ret = 0; uint32_t max_wq_data; MLX5_ASSERT(txq_data); @@ -505,7 +504,7 @@ "port %u Tx queue %u cannot allocate memory resources", dev->data->port_id, txq_data->idx); rte_errno = ENOMEM; - goto error; + return NULL; } tmpl->type = MLX5_TXQ_OBJ_TYPE_DEVX_HAIRPIN; tmpl->txq_ctrl = txq_ctrl; @@ -518,6 +517,7 @@ DRV_LOG(ERR, "total data size %u power of 2 is " "too large for hairpin", priv->config.log_hp_size); + rte_free(tmpl); rte_errno = ERANGE; return NULL; } @@ -537,22 +537,15 @@ DRV_LOG(ERR, "port %u tx hairpin queue %u can't create sq object", dev->data->port_id, idx); + rte_free(tmpl); rte_errno = errno; - goto error; + return NULL; } DRV_LOG(DEBUG, "port %u sxq %u updated with %p", dev->data->port_id, idx, (void *)&tmpl); rte_atomic32_inc(&tmpl->refcnt); LIST_INSERT_HEAD(&priv->txqsobj, tmpl, next); return tmpl; -error: - ret = rte_errno; /* Save rte_errno before cleanup. */ - if (tmpl->tis) - mlx5_devx_cmd_destroy(tmpl->tis); - if (tmpl->sq) - mlx5_devx_cmd_destroy(tmpl->sq); - rte_errno = ret; /* Restore rte_errno. */ - return NULL; } /**