mbox series

[00/20] fix memory leaks in error handling

Message ID tencent_3C112E11A018311EA7D3D908771D89EFBF07@qq.com (mailing list archive)
Headers
Series fix memory leaks in error handling |

Message

Weiguo Li Feb. 22, 2022, 6:17 p.m. UTC
  This series fix some memory leaks in error handling.

I write a coccinelle script to detect these issues, and
fix them after exclude a small number of false positives.

FYI, the script is as follows:
//
// Find possible memory leaks in error handling
// Reference: coccinellery/alloc9/kmalloc9.cocci
//
@memory_leak_in_error_handling @
local idexpression x;
expression E;
statement S;
identifier dpdk_malloc = { rte_malloc, rte_zmalloc, rte_realloc, rte_malloc_socket,
    rte_zmalloc_socket, rte_realloc_socket, rte_pktmbuf_alloc, opae_malloc,
    strdup, strndup, malloc, realloc, calloc };
identifier dpdk_free = { rte_free, free, bnx2x_rx_queue_release, bnx2x_tx_queue_release,
    rte_pktmbuf_free, opae_free, ntb_rxq_release, ntb_txq_release, ice_rx_queue_release,
    ice_tx_queue_release };
type T;
@@
(
  /* skip this pattern to reduce false positives */
  x = dpdk_malloc(...); ... if (...) { ... \( return x; \| return 0; \) }
|
* x = dpdk_malloc(...);
  ...
  if (x == NULL) S
  ... when != E = x
      when != dpdk_free (..., \( x \| (T)x \| &x \), ...)
      when forall
  if (...) {
    <+... when != E = x
          when != dpdk_free (..., \( x \| (T)x \| &x \), ...)
          when forall
*   return ...;
    ...+>
  }
)

Weiguo Li (20):
  baseband/acc100: fix a memory leak in acc100 queue setup
  common/dpaax: fix a memory leak in iterate dir
  crypto/dpaa2_sec: fix memory leaks in error handlings
  crypto/qat: fix a memory leak when set encrypt key fail
  net/bnxt: fix a memory leak in error handling
  net/bnxt: fix 'ctx' memory leak when new malloc fail
  net/bnx2x: add clean up for 'rxq' to avoid a memory leak
  net/cnxk: free 'node' memory when node add fail
  net/dpaa: fix a memory leak when validation fail
  net/failsafe: fix a memory leak in error handling
  net/iavf: fix a memory leak in error handling
  net/ice: goto clean up lable to avoid memory leak
  net/ice: fix memory leaks in error handlings
  net/ice: avoid fix memory leaks in register parser
  net/memif: fix some memory leaks in error handlings
  net/sfc: fix a memory leak in error handling
  net/vmxnet3: fix memory leaks in error handlings
  raw/ifpga/base: fix memory leaks in error handlings
  raw/ntb: fix some memory leaks in error handlings
  regex/mlx5: fix a memory leak in error handling

 drivers/baseband/acc100/rte_acc100_pmd.c    |  8 +++---
 drivers/common/dpaax/dpaa_of.c              |  4 ++-
 drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c |  3 ++
 drivers/crypto/qat/qat_sym_session.c        |  9 +++---
 drivers/net/bnx2x/bnx2x_rxtx.c              |  1 +
 drivers/net/bnxt/bnxt_hwrm.c                |  1 +
 drivers/net/bnxt/tf_ulp/ulp_fc_mgr.c        |  1 +
 drivers/net/cnxk/cnxk_tm.c                  |  1 +
 drivers/net/dpaa/dpaa_rxtx.c                |  1 +
 drivers/net/failsafe/failsafe_ops.c         |  1 +
 drivers/net/iavf/iavf_generic_flow.c        |  1 +
 drivers/net/ice/ice_acl_filter.c            |  2 +-
 drivers/net/ice/ice_generic_flow.c          |  9 +++---
 drivers/net/ice/ice_hash.c                  | 30 ++++++++++++-------
 drivers/net/memif/rte_eth_memif.c           | 32 ++++++++++++++-------
 drivers/net/sfc/sfc.c                       |  4 ++-
 drivers/net/vmxnet3/vmxnet3_rxtx.c          |  8 ++++++
 drivers/raw/ifpga/base/ifpga_enumerate.c    | 10 +++++--
 drivers/raw/ifpga/base/opae_eth_group.c     |  1 +
 drivers/raw/ifpga/base/opae_i2c.c           |  5 +++-
 drivers/raw/ntb/ntb.c                       |  9 +++---
 drivers/regex/mlx5/mlx5_rxp.c               |  4 ++-
 22 files changed, 100 insertions(+), 45 deletions(-)
  

Comments

Ajit Khaparde Feb. 23, 2022, 2:36 a.m. UTC | #1
On Tue, Feb 22, 2022 at 10:18 AM Weiguo Li <liwg06@foxmail.com> wrote:
>
> This series fix some memory leaks in error handling.
>
> I write a coccinelle script to detect these issues, and
> fix them after exclude a small number of false positives.
>
> FYI, the script is as follows:
> //
> // Find possible memory leaks in error handling
> // Reference: coccinellery/alloc9/kmalloc9.cocci
> //
> @memory_leak_in_error_handling @
> local idexpression x;
> expression E;
> statement S;
> identifier dpdk_malloc = { rte_malloc, rte_zmalloc, rte_realloc, rte_malloc_socket,
>     rte_zmalloc_socket, rte_realloc_socket, rte_pktmbuf_alloc, opae_malloc,
>     strdup, strndup, malloc, realloc, calloc };
> identifier dpdk_free = { rte_free, free, bnx2x_rx_queue_release, bnx2x_tx_queue_release,
>     rte_pktmbuf_free, opae_free, ntb_rxq_release, ntb_txq_release, ice_rx_queue_release,
>     ice_tx_queue_release };
> type T;
> @@
> (
>   /* skip this pattern to reduce false positives */
>   x = dpdk_malloc(...); ... if (...) { ... \( return x; \| return 0; \) }
> |
> * x = dpdk_malloc(...);
>   ...
>   if (x == NULL) S
>   ... when != E = x
>       when != dpdk_free (..., \( x \| (T)x \| &x \), ...)
>       when forall
>   if (...) {
>     <+... when != E = x
>           when != dpdk_free (..., \( x \| (T)x \| &x \), ...)
>           when forall
> *   return ...;
>     ...+>
>   }
> )
>
> Weiguo Li (20):
>   baseband/acc100: fix a memory leak in acc100 queue setup
>   common/dpaax: fix a memory leak in iterate dir
>   crypto/dpaa2_sec: fix memory leaks in error handlings
>   crypto/qat: fix a memory leak when set encrypt key fail
>   net/bnxt: fix a memory leak in error handling
>   net/bnxt: fix 'ctx' memory leak when new malloc fail
>   net/bnx2x: add clean up for 'rxq' to avoid a memory leak
>   net/cnxk: free 'node' memory when node add fail
>   net/dpaa: fix a memory leak when validation fail
>   net/failsafe: fix a memory leak in error handling
>   net/iavf: fix a memory leak in error handling
>   net/ice: goto clean up lable to avoid memory leak
>   net/ice: fix memory leaks in error handlings
>   net/ice: avoid fix memory leaks in register parser
>   net/memif: fix some memory leaks in error handlings
>   net/sfc: fix a memory leak in error handling
>   net/vmxnet3: fix memory leaks in error handlings
>   raw/ifpga/base: fix memory leaks in error handlings
>   raw/ntb: fix some memory leaks in error handlings
>   regex/mlx5: fix a memory leak in error handling
>
>  drivers/baseband/acc100/rte_acc100_pmd.c    |  8 +++---
>  drivers/common/dpaax/dpaa_of.c              |  4 ++-
>  drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c |  3 ++
>  drivers/crypto/qat/qat_sym_session.c        |  9 +++---
>  drivers/net/bnx2x/bnx2x_rxtx.c              |  1 +
>  drivers/net/bnxt/bnxt_hwrm.c                |  1 +
>  drivers/net/bnxt/tf_ulp/ulp_fc_mgr.c        |  1 +
ACK for bnxt patches. Thanks


>  drivers/net/cnxk/cnxk_tm.c                  |  1 +
>  drivers/net/dpaa/dpaa_rxtx.c                |  1 +
>  drivers/net/failsafe/failsafe_ops.c         |  1 +
>  drivers/net/iavf/iavf_generic_flow.c        |  1 +
>  drivers/net/ice/ice_acl_filter.c            |  2 +-
>  drivers/net/ice/ice_generic_flow.c          |  9 +++---
>  drivers/net/ice/ice_hash.c                  | 30 ++++++++++++-------
>  drivers/net/memif/rte_eth_memif.c           | 32 ++++++++++++++-------
>  drivers/net/sfc/sfc.c                       |  4 ++-
>  drivers/net/vmxnet3/vmxnet3_rxtx.c          |  8 ++++++
>  drivers/raw/ifpga/base/ifpga_enumerate.c    | 10 +++++--
>  drivers/raw/ifpga/base/opae_eth_group.c     |  1 +
>  drivers/raw/ifpga/base/opae_i2c.c           |  5 +++-
>  drivers/raw/ntb/ntb.c                       |  9 +++---
>  drivers/regex/mlx5/mlx5_rxp.c               |  4 ++-
>  22 files changed, 100 insertions(+), 45 deletions(-)
>
> --
> 2.25.1
>
  
David Marchand June 2, 2022, 8:06 a.m. UTC | #2
On Tue, Feb 22, 2022 at 7:18 PM Weiguo Li <liwg06@foxmail.com> wrote:
>
> This series fix some memory leaks in error handling.
>
> I write a coccinelle script to detect these issues, and
> fix them after exclude a small number of false positives.
>
> FYI, the script is as follows:
> //
> // Find possible memory leaks in error handling
> // Reference: coccinellery/alloc9/kmalloc9.cocci
> //
> @memory_leak_in_error_handling @
> local idexpression x;
> expression E;
> statement S;
> identifier dpdk_malloc = { rte_malloc, rte_zmalloc, rte_realloc, rte_malloc_socket,
>     rte_zmalloc_socket, rte_realloc_socket, rte_pktmbuf_alloc, opae_malloc,
>     strdup, strndup, malloc, realloc, calloc };
> identifier dpdk_free = { rte_free, free, bnx2x_rx_queue_release, bnx2x_tx_queue_release,
>     rte_pktmbuf_free, opae_free, ntb_rxq_release, ntb_txq_release, ice_rx_queue_release,
>     ice_tx_queue_release };
> type T;
> @@
> (
>   /* skip this pattern to reduce false positives */
>   x = dpdk_malloc(...); ... if (...) { ... \( return x; \| return 0; \) }
> |
> * x = dpdk_malloc(...);
>   ...
>   if (x == NULL) S
>   ... when != E = x
>       when != dpdk_free (..., \( x \| (T)x \| &x \), ...)
>       when forall
>   if (...) {
>     <+... when != E = x
>           when != dpdk_free (..., \( x \| (T)x \| &x \), ...)
>           when forall
> *   return ...;
>     ...+>
>   }
> )

Thanks for those who did reviews, but there are a lot of unattended
fixes in this series.
Reviews please!
  
David Marchand June 24, 2022, 9:04 p.m. UTC | #3
On Tue, Feb 22, 2022 at 7:18 PM Weiguo Li <liwg06@foxmail.com> wrote:
>
> This series fix some memory leaks in error handling.
>
> I write a coccinelle script to detect these issues, and
> fix them after exclude a small number of false positives.
>
> FYI, the script is as follows:
> //
> // Find possible memory leaks in error handling
> // Reference: coccinellery/alloc9/kmalloc9.cocci
> //
> @memory_leak_in_error_handling @
> local idexpression x;
> expression E;
> statement S;
> identifier dpdk_malloc = { rte_malloc, rte_zmalloc, rte_realloc, rte_malloc_socket,
>     rte_zmalloc_socket, rte_realloc_socket, rte_pktmbuf_alloc, opae_malloc,
>     strdup, strndup, malloc, realloc, calloc };
> identifier dpdk_free = { rte_free, free, bnx2x_rx_queue_release, bnx2x_tx_queue_release,
>     rte_pktmbuf_free, opae_free, ntb_rxq_release, ntb_txq_release, ice_rx_queue_release,
>     ice_tx_queue_release };
> type T;
> @@
> (
>   /* skip this pattern to reduce false positives */
>   x = dpdk_malloc(...); ... if (...) { ... \( return x; \| return 0; \) }
> |
> * x = dpdk_malloc(...);
>   ...
>   if (x == NULL) S
>   ... when != E = x
>       when != dpdk_free (..., \( x \| (T)x \| &x \), ...)
>       when forall
>   if (...) {
>     <+... when != E = x
>           when != dpdk_free (..., \( x \| (T)x \| &x \), ...)
>           when forall
> *   return ...;
>     ...+>
>   }
> )
>
> Weiguo Li (20):
>   baseband/acc100: fix a memory leak in acc100 queue setup
>   common/dpaax: fix a memory leak in iterate dir
>   crypto/dpaa2_sec: fix memory leaks in error handlings
>   crypto/qat: fix a memory leak when set encrypt key fail
>   net/bnxt: fix a memory leak in error handling
>   net/bnxt: fix 'ctx' memory leak when new malloc fail
>   net/bnx2x: add clean up for 'rxq' to avoid a memory leak
>   net/cnxk: free 'node' memory when node add fail
>   net/dpaa: fix a memory leak when validation fail
>   net/failsafe: fix a memory leak in error handling
>   net/iavf: fix a memory leak in error handling
>   net/ice: goto clean up lable to avoid memory leak
>   net/ice: fix memory leaks in error handlings
>   net/ice: avoid fix memory leaks in register parser
>   net/memif: fix some memory leaks in error handlings
>   net/sfc: fix a memory leak in error handling
>   net/vmxnet3: fix memory leaks in error handlings
>   raw/ifpga/base: fix memory leaks in error handlings
>   raw/ntb: fix some memory leaks in error handlings
>   regex/mlx5: fix a memory leak in error handling
>
>  drivers/baseband/acc100/rte_acc100_pmd.c    |  8 +++---
>  drivers/common/dpaax/dpaa_of.c              |  4 ++-
>  drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c |  3 ++
>  drivers/crypto/qat/qat_sym_session.c        |  9 +++---
>  drivers/net/bnx2x/bnx2x_rxtx.c              |  1 +
>  drivers/net/bnxt/bnxt_hwrm.c                |  1 +
>  drivers/net/bnxt/tf_ulp/ulp_fc_mgr.c        |  1 +
>  drivers/net/cnxk/cnxk_tm.c                  |  1 +
>  drivers/net/dpaa/dpaa_rxtx.c                |  1 +
>  drivers/net/failsafe/failsafe_ops.c         |  1 +
>  drivers/net/iavf/iavf_generic_flow.c        |  1 +
>  drivers/net/ice/ice_acl_filter.c            |  2 +-
>  drivers/net/ice/ice_generic_flow.c          |  9 +++---
>  drivers/net/ice/ice_hash.c                  | 30 ++++++++++++-------
>  drivers/net/memif/rte_eth_memif.c           | 32 ++++++++++++++-------
>  drivers/net/sfc/sfc.c                       |  4 ++-
>  drivers/net/vmxnet3/vmxnet3_rxtx.c          |  8 ++++++
>  drivers/raw/ifpga/base/ifpga_enumerate.c    | 10 +++++--
>  drivers/raw/ifpga/base/opae_eth_group.c     |  1 +
>  drivers/raw/ifpga/base/opae_i2c.c           |  5 +++-
>  drivers/raw/ntb/ntb.c                       |  9 +++---
>  drivers/regex/mlx5/mlx5_rxp.c               |  4 ++-
>  22 files changed, 100 insertions(+), 45 deletions(-)

There is some rebase needed: at least one patch on net/ice does not
apply anymore, and I see that the fix on crypto/dpaa2_sec needs a
rebase too to be complete.
I only looked at the first 5 patches, and sent some comments.
I don't think the rest of the patches will be different, I expect more
comments or doubts from me.

It is already late here, I won't take this series for -rc2, sorry.

To make this series progress, I suggest rebasing, and sending per
driver series, so that those series land in the relevant subtrees.


Thanks.