Re: [PATCH] crypto: caam - free qman_fq after kill_fq

[Horia Geantă]

On 7/11/2017 9:21 AM, Xulin Sun wrote:
> kill_fq removes a complete frame queue, it needs to free the qman_fq
> in the last. Else kmemleak will report the below warning:
> 
> unreferenced object 0x800073085c80 (size 128):
>   comm "cryptomgr_test", pid 199, jiffies 4294937850 (age 67.840s)
>   hex dump (first 32 bytes):
> 00 00 00 00 00 00 00 00 a0 80 7e 00 00 80 ff ff
> 00 00 00 00 00 00 00 00 04 00 04 00 5c 01 00 00
>   backtrace:
> [] create_object+0xf8/0x258
> [] kmemleak_alloc+0x58/0xa0
> [] kmem_cache_alloc_trace+0x2c8/0x358
> [] create_caam_req_fq+0x40/0x170
> [] caam_drv_ctx_update+0x54/0x248
> [] aead_setkey+0x154/0x300
> [] setkey+0x50/0xf0
> [] __test_aead+0x5ec/0x1028
> [] test_aead+0x44/0xc8
> [] alg_test_aead+0x58/0xd0
> [] alg_test+0x14c/0x308
> [] cryptomgr_test+0x50/0x58
> [] kthread+0xdc/0xf0
> [] ret_from_fork+0x10/0x50
> 
> And check where the function kill_fq() is called to remove
> the additional kfree to qman_fq.
> 
> Signed-off-by: Xulin Sun 
> ---
>  drivers/crypto/caam/qi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/crypto/caam/qi.c b/drivers/crypto/caam/qi.c
> index 1990ed4..c4b9173 100644
> --- a/drivers/crypto/caam/qi.c
> +++ b/drivers/crypto/caam/qi.c
> @@ -277,6 +277,7 @@ static int kill_fq(struct device *qidev, struct qman_fq 
> *fq)
>   dev_err(qidev, "OOS of FQID: %u failed\n", fq->fqid);
>  
>   qman_destroy_fq(fq);
> + kfree(fq);

There is a case where kfree(fq) is executed and kill_fq() returns != 0 -
that is when qman_oos_fq() returns != 0.

Thus please make sure there is no reference to "fq" after kill_fq(...,
fq) is called, even if kill_fq() return code is != 0.
For e.g.:
if (kill_fq(qidev, new_fq))
dev_warn(qidev, "New CAAM FQ: %u kill failed\n",
 new_fq->fqid);
 ^^ already kfree-ed

Thanks,
Horia

>  
>   return ret;
>  }
> @@ -511,7 +512,6 @@ int caam_qi_shutdown(struct device *qidev)
>  
>   if (kill_fq(qidev, per_cpu(pcpu_qipriv.rsp_fq, i)))
>   dev_err(qidev, "Rsp FQ kill failed, cpu: %d\n", i);
> - kfree(per_cpu(pcpu_qipriv.rsp_fq, i));
>   }
>  
>   /*
> 

[PATCH] crypto: caam - free qman_fq after kill_fq

[Xulin Sun]

kill_fq removes a complete frame queue, it needs to free the qman_fq
in the last. Else kmemleak will report the below warning:

unreferenced object 0x800073085c80 (size 128):
  comm "cryptomgr_test", pid 199, jiffies 4294937850 (age 67.840s)
  hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 a0 80 7e 00 00 80 ff ff
00 00 00 00 00 00 00 00 04 00 04 00 5c 01 00 00
  backtrace:
[] create_object+0xf8/0x258
[] kmemleak_alloc+0x58/0xa0
[] kmem_cache_alloc_trace+0x2c8/0x358
[] create_caam_req_fq+0x40/0x170
[] caam_drv_ctx_update+0x54/0x248
[] aead_setkey+0x154/0x300
[] setkey+0x50/0xf0
[] __test_aead+0x5ec/0x1028
[] test_aead+0x44/0xc8
[] alg_test_aead+0x58/0xd0
[] alg_test+0x14c/0x308
[] cryptomgr_test+0x50/0x58
[] kthread+0xdc/0xf0
[] ret_from_fork+0x10/0x50

And check where the function kill_fq() is called to remove
the additional kfree to qman_fq.

Signed-off-by: Xulin Sun 
---
 drivers/crypto/caam/qi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/caam/qi.c b/drivers/crypto/caam/qi.c
index 1990ed4..c4b9173 100644
--- a/drivers/crypto/caam/qi.c
+++ b/drivers/crypto/caam/qi.c
@@ -277,6 +277,7 @@ static int kill_fq(struct device *qidev, struct qman_fq *fq)
dev_err(qidev, "OOS of FQID: %u failed\n", fq->fqid);
 
qman_destroy_fq(fq);
+   kfree(fq);
 
return ret;
 }
@@ -511,7 +512,6 @@ int caam_qi_shutdown(struct device *qidev)
 
if (kill_fq(qidev, per_cpu(pcpu_qipriv.rsp_fq, i)))
dev_err(qidev, "Rsp FQ kill failed, cpu: %d\n", i);
-   kfree(per_cpu(pcpu_qipriv.rsp_fq, i));
}
 
/*
-- 
2.7.4