crypto/ccp: Check for the NULL pointer after calling rte_malloc

Message ID tencent_5A1578430CBABE04584679251A5D4910BE06@qq.com (mailing list archive)
State Changes Requested, archived
Delegated to: akhil goyal
Headers
Series crypto/ccp: Check for the NULL pointer after calling rte_malloc |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS

Commit Message

biggest dreamer July 9, 2022, 11:01 a.m. UTC
  From: Shiqi Liu <835703180@qq.com>

As the possible failure of the rte_malloc(), the not_checked and
checked could be NULL pointer.
Therefore, it should be better to check it in order to avoid
the dereference of the NULL pointer.

Fixes: 09a0fd736a0 ("crypto/ccp: enable IOMMU")
Signed-off-by: Shiqi Liu <835703180@qq.com>
---
 drivers/crypto/ccp/rte_ccp_pmd.c | 3 +++
 1 file changed, 3 insertions(+)
  

Comments

Namburu, Chandu-babu July 20, 2022, 6:29 a.m. UTC | #1
[Public]

Acked-by: Chandubabu Namburu <chandu@amd.com>

-----Original Message-----
From: 835703180@qq.com <835703180@qq.com> 
Sent: Saturday, July 9, 2022 4:31 PM
To: Namburu, Chandu-babu <chandu@amd.com>
Cc: dev@dpdk.org; Shiqi Liu <835703180@qq.com>
Subject: [PATCH] crypto/ccp: Check for the NULL pointer after calling rte_malloc

From: Shiqi Liu <835703180@qq.com>

As the possible failure of the rte_malloc(), the not_checked and checked could be NULL pointer.
Therefore, it should be better to check it in order to avoid the dereference of the NULL pointer.

Fixes: 09a0fd736a0 ("crypto/ccp: enable IOMMU")
Signed-off-by: Shiqi Liu <835703180@qq.com>
---
 drivers/crypto/ccp/rte_ccp_pmd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/crypto/ccp/rte_ccp_pmd.c b/drivers/crypto/ccp/rte_ccp_pmd.c
index a35a8cd775..776f928864 100644
--- a/drivers/crypto/ccp/rte_ccp_pmd.c
+++ b/drivers/crypto/ccp/rte_ccp_pmd.c
@@ -301,6 +301,9 @@ cryptodev_ccp_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	};
 
 	sha_ctx = (void *)rte_malloc(NULL, SHA512_DIGEST_SIZE, 64);
+	if (sha_ctx == NULL) {
+		return -ENOMEM;
+	}
 	if (ccp_pmd_init_done) {
 		RTE_LOG(INFO, PMD, "CCP PMD already initialized\n");
 		return -EFAULT;
--
2.35.1.windows.2
  
David Marchand July 20, 2022, 1:43 p.m. UTC | #2
On Wed, Jul 20, 2022 at 8:29 AM Namburu, Chandu-babu <chandu@amd.com> wrote:
> From: Shiqi Liu <835703180@qq.com>
>
> As the possible failure of the rte_malloc(), the not_checked and checked could be NULL pointer.
> Therefore, it should be better to check it in order to avoid the dereference of the NULL pointer.
>
> Fixes: 09a0fd736a0 ("crypto/ccp: enable IOMMU")
> Signed-off-by: Shiqi Liu <835703180@qq.com>

This sha_ctx variable and its accesses are suspicious.

It seems to be used as some kind of intermediate buffer, but I fail to
see the need.
Can't the existing code rely on sess->auth.ctx ?

There is also a suspicious mention (in ccp_perform_sha) of sha_ctx but
with no calling rte_mem_virt2iova().
  
Stephen Hemminger July 20, 2022, 3:42 p.m. UTC | #3
On Wed, 20 Jul 2022 06:29:06 +0000
"Namburu, Chandu-babu" <chandu@amd.com> wrote:

>  	sha_ctx = (void *)rte_malloc(NULL, SHA512_DIGEST_SIZE, 64);
> +	if (sha_ctx == NULL) {
> +		return -ENOMEM;
> +	}

There is unnecessary cast here (pre-existing).

rte_malloc() already returns void *
  
Akhil Goyal Aug. 16, 2022, 4:01 p.m. UTC | #4
Hi,
Could you please reply to David and Stephen's comments?

Regards,
Akhil
> On Wed, Jul 20, 2022 at 8:29 AM Namburu, Chandu-babu <chandu@amd.com>
> wrote:
> > From: Shiqi Liu <835703180@qq.com>
> >
> > As the possible failure of the rte_malloc(), the not_checked and checked could
> be NULL pointer.
> > Therefore, it should be better to check it in order to avoid the dereference of
> the NULL pointer.
> >
> > Fixes: 09a0fd736a0 ("crypto/ccp: enable IOMMU")
> > Signed-off-by: Shiqi Liu <835703180@qq.com>
> 
> This sha_ctx variable and its accesses are suspicious.
> 
> It seems to be used as some kind of intermediate buffer, but I fail to
> see the need.
> Can't the existing code rely on sess->auth.ctx ?
> 
> There is also a suspicious mention (in ccp_perform_sha) of sha_ctx but
> with no calling rte_mem_virt2iova().
> 
> 
> --
> David Marchand
  
Namburu, Chandu-babu Aug. 23, 2022, 6:14 a.m. UTC | #5
[Public]

+ sunil

-----Original Message-----
From: Akhil Goyal <gakhil@marvell.com> 
Sent: Tuesday, August 16, 2022 9:31 PM
To: Namburu, Chandu-babu <chandu@amd.com>; 835703180@qq.com
Cc: dev@dpdk.org; David Marchand <david.marchand@redhat.com>
Subject: RE: [EXT] Re: [PATCH] crypto/ccp: Check for the NULL pointer after calling rte_malloc

Hi,
Could you please reply to David and Stephen's comments?

Regards,
Akhil
> On Wed, Jul 20, 2022 at 8:29 AM Namburu, Chandu-babu <chandu@amd.com>
> wrote:
> > From: Shiqi Liu <835703180@qq.com>
> >
> > As the possible failure of the rte_malloc(), the not_checked and 
> > checked could
> be NULL pointer.
> > Therefore, it should be better to check it in order to avoid the 
> > dereference of
> the NULL pointer.
> >
> > Fixes: 09a0fd736a0 ("crypto/ccp: enable IOMMU")
> > Signed-off-by: Shiqi Liu <835703180@qq.com>
> 
> This sha_ctx variable and its accesses are suspicious.
> 
> It seems to be used as some kind of intermediate buffer, but I fail to 
> see the need.
> Can't the existing code rely on sess->auth.ctx ?
> 
> There is also a suspicious mention (in ccp_perform_sha) of sha_ctx but 
> with no calling rte_mem_virt2iova().
> 
> 
> --
> David Marchand
  

Patch

diff --git a/drivers/crypto/ccp/rte_ccp_pmd.c b/drivers/crypto/ccp/rte_ccp_pmd.c
index a35a8cd775..776f928864 100644
--- a/drivers/crypto/ccp/rte_ccp_pmd.c
+++ b/drivers/crypto/ccp/rte_ccp_pmd.c
@@ -301,6 +301,9 @@  cryptodev_ccp_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	};
 
 	sha_ctx = (void *)rte_malloc(NULL, SHA512_DIGEST_SIZE, 64);
+	if (sha_ctx == NULL) {
+		return -ENOMEM;
+	}
 	if (ccp_pmd_init_done) {
 		RTE_LOG(INFO, PMD, "CCP PMD already initialized\n");
 		return -EFAULT;