compress/isal: fix use after free

Message ID 20190521144713.12046-1-stephen@networkplumber.org (mailing list archive)
State Accepted, archived
Delegated to: akhil goyal
Headers
Series compress/isal: fix use after free |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS

Commit Message

Stephen Hemminger May 21, 2019, 2:47 p.m. UTC
  The release function was using qp->stream after already
releasing it and the null pointer checking was missing.

Also since rte_free(NULL) is a no-op, remove unnecessary
checks for NULL.

Coverity issure: 340860
Fixes: dc49e6aa4879 ("compress/isal: add ISA-L compression functionality")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/compress/isal/isal_compress_pmd_ops.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)
  

Comments

Daly, Lee May 21, 2019, 3:07 p.m. UTC | #1
Hi Stephen, 
Thanks for the patch.

> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Tuesday, May 21, 2019 3:47 PM
> To: Daly, Lee <lee.daly@intel.com>
> Cc: dev@dpdk.org; Stephen Hemminger <stephen@networkplumber.org>
> Subject: [PATCH] compress/isal: fix use after free
> 
> The release function was using qp->stream after already releasing it and the
> null pointer checking was missing.
> 
> Also since rte_free(NULL) is a no-op, remove unnecessary checks for NULL.
> 
> Coverity issure: 340860
> Fixes: dc49e6aa4879 ("compress/isal: add ISA-L compression functionality")
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  drivers/compress/isal/isal_compress_pmd_ops.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/compress/isal/isal_compress_pmd_ops.c

 Acked-by: Lee Daly <lee.daly@intel.com>
  
Fiona Trahe May 27, 2019, 4 p.m. UTC | #2
> -----Original Message-----
> From: Daly, Lee
> Sent: Tuesday, May 21, 2019 4:08 PM
> To: Stephen Hemminger <stephen@networkplumber.org>
> Cc: dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>
> Subject: RE: [PATCH] compress/isal: fix use after free
> 
> Hi Stephen,
> Thanks for the patch.
> 
> > -----Original Message-----
> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > Sent: Tuesday, May 21, 2019 3:47 PM
> > To: Daly, Lee <lee.daly@intel.com>
> > Cc: dev@dpdk.org; Stephen Hemminger <stephen@networkplumber.org>
> > Subject: [PATCH] compress/isal: fix use after free
> >
> > The release function was using qp->stream after already releasing it and the
> > null pointer checking was missing.
> >
> > Also since rte_free(NULL) is a no-op, remove unnecessary checks for NULL.
> >
> > Coverity issure: 340860
> > Fixes: dc49e6aa4879 ("compress/isal: add ISA-L compression functionality")
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
Acked-by: Lee Daly <lee.daly@intel.com>
Acked-by: Fiona Trahe <fiona.trahe@intel.com>
  
Akhil Goyal June 19, 2019, 2:54 p.m. UTC | #3
> > >
> > > The release function was using qp->stream after already releasing it and the
> > > null pointer checking was missing.
> > >
> > > Also since rte_free(NULL) is a no-op, remove unnecessary checks for NULL.
> > >
> > > Coverity issure: 340860
> > > Fixes: dc49e6aa4879 ("compress/isal: add ISA-L compression functionality")
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > ---
> Acked-by: Lee Daly <lee.daly@intel.com>
> Acked-by: Fiona Trahe <fiona.trahe@intel.com>

Applied to dpdk-next-crypto

Thanks.
  

Patch

diff --git a/drivers/compress/isal/isal_compress_pmd_ops.c b/drivers/compress/isal/isal_compress_pmd_ops.c
index fe9995992304..77ac6fcf21fc 100644
--- a/drivers/compress/isal/isal_compress_pmd_ops.c
+++ b/drivers/compress/isal/isal_compress_pmd_ops.c
@@ -171,18 +171,12 @@  isal_comp_pmd_qp_release(struct rte_compressdev *dev, uint16_t qp_id)
 	if (qp == NULL)
 		return -EINVAL;
 
-	if (qp->stream != NULL)
-		rte_free(qp->stream);
-
-	if (qp->stream->level_buf != NULL)
+	if (qp->stream)
 		rte_free(qp->stream->level_buf);
 
-	if (qp->state != NULL)
-		rte_free(qp->state);
-
-	if (qp->processed_pkts != NULL)
-		rte_ring_free(qp->processed_pkts);
-
+	rte_free(qp->state);
+	rte_ring_free(qp->processed_pkts);
+	rte_free(qp->stream);
 	rte_free(qp);
 	dev->data->queue_pairs[qp_id] = NULL;