diff mbox series

[01/20] baseband/acc100: fix a memory leak in acc100 queue setup

Message ID tencent_EAF6EFC9911811CF98099EEC539908751D07@qq.com (mailing list archive)
State Changes Requested, archived
Delegated to: David Marchand
Headers
Series fix memory leaks in error handling |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Weiguo Li Feb. 22, 2022, 6:17 p.m. UTC 
  We allocated memory for 'q', we don't free it when null check for 'd' fails
and it will lead to memory leak.
We can move null check for 'd' ahead of the memory allocation to fix it.

Fixes: 060e76729302 ("baseband/acc100: add queue configuration")

Signed-off-by: Weiguo Li <liwg06@foxmail.com>
---
 drivers/baseband/acc100/rte_acc100_pmd.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Chautru, Nicolas Feb. 23, 2022, 5:42 p.m. UTC | #1 
> From: Weiguo Li <liwg06@foxmail.com>
> 
> We allocated memory for 'q', we don't free it when null check for 'd' fails and
> it will lead to memory leak.
> We can move null check for 'd' ahead of the memory allocation to fix it.
> 
> Fixes: 060e76729302 ("baseband/acc100: add queue configuration")
> 
> Signed-off-by: Weiguo Li <liwg06@foxmail.com>
> ---
>  drivers/baseband/acc100/rte_acc100_pmd.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c
> b/drivers/baseband/acc100/rte_acc100_pmd.c
> index f86474f7e0..25e9e6435f 100644
> --- a/drivers/baseband/acc100/rte_acc100_pmd.c
> +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
> @@ -824,6 +824,10 @@ acc100_queue_setup(struct rte_bbdev *dev,
> uint16_t queue_id,
>  	struct acc100_queue *q;
>  	int16_t q_idx;
> 
> +	if (d == NULL) {
> +		rte_bbdev_log(ERR, "Undefined device");
> +		return -ENODEV;
> +	}
>  	/* Allocate the queue data structure. */
>  	q = rte_zmalloc_socket(dev->device->driver->name, sizeof(*q),
>  			RTE_CACHE_LINE_SIZE, conf->socket);
> @@ -831,10 +835,6 @@ acc100_queue_setup(struct rte_bbdev *dev,
> uint16_t queue_id,
>  		rte_bbdev_log(ERR, "Failed to allocate queue memory");
>  		return -ENOMEM;
>  	}
> -	if (d == NULL) {
> -		rte_bbdev_log(ERR, "Undefined device");
> -		return -ENODEV;
> -	}
> 
>  	q->d = d;
>  	q->ring_addr = RTE_PTR_ADD(d->sw_rings, (d->sw_ring_size *
> queue_id));
> --
> 2.25.1
> 

Thanks

Acked-by: Nicolas Chautru <nicolas.chautr@intel.com>
David Marchand June 24, 2022, 8:45 p.m. UTC | #2 
On Tue, Feb 22, 2022 at 7:18 PM Weiguo Li <liwg06@foxmail.com> wrote:
>
> We allocated memory for 'q', we don't free it when null check for 'd' fails
> and it will lead to memory leak.
> We can move null check for 'd' ahead of the memory allocation to fix it.
>
> Fixes: 060e76729302 ("baseband/acc100: add queue configuration")
>
> Signed-off-by: Weiguo Li <liwg06@foxmail.com>
> ---
>  drivers/baseband/acc100/rte_acc100_pmd.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c b/drivers/baseband/acc100/rte_acc100_pmd.c
> index f86474f7e0..25e9e6435f 100644
> --- a/drivers/baseband/acc100/rte_acc100_pmd.c
> +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
> @@ -824,6 +824,10 @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
>         struct acc100_queue *q;
>         int16_t q_idx;
>
> +       if (d == NULL) {
> +               rte_bbdev_log(ERR, "Undefined device");
> +               return -ENODEV;
> +       }

Nicolas,

.queue_setup is a dev_ops, which means it is invoked after the .probe
function was called.
Failing to allocate dev_private shoud have been detected earlier, is
that correct?
If so, this check should simply be dropped, and there is no leak to fix.

Please confirm when you have the time.

Thanks.
diff mbox series

Patch

diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c b/drivers/baseband/acc100/rte_acc100_pmd.c
index f86474f7e0..25e9e6435f 100644
--- a/drivers/baseband/acc100/rte_acc100_pmd.c
+++ b/drivers/baseband/acc100/rte_acc100_pmd.c
@@ -824,6 +824,10 @@  acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
 	struct acc100_queue *q;
 	int16_t q_idx;
 
+	if (d == NULL) {
+		rte_bbdev_log(ERR, "Undefined device");
+		return -ENODEV;
+	}
 	/* Allocate the queue data structure. */
 	q = rte_zmalloc_socket(dev->device->driver->name, sizeof(*q),
 			RTE_CACHE_LINE_SIZE, conf->socket);
@@ -831,10 +835,6 @@  acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
 		rte_bbdev_log(ERR, "Failed to allocate queue memory");
 		return -ENOMEM;
 	}
-	if (d == NULL) {
-		rte_bbdev_log(ERR, "Undefined device");
-		return -ENODEV;
-	}
 
 	q->d = d;
 	q->ring_addr = RTE_PTR_ADD(d->sw_rings, (d->sw_ring_size * queue_id));