[01/20] baseband/acc100: fix a memory leak in acc100 queue setup
Checks
Commit Message
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
> 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>
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.
@@ -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));