Re: [PATCH] test/security: fix buffer leaks in error path

Fri, 25 Aug 2023 04:23:09 -0700 


On 22-Aug-23 11:03 PM, Akhil Goyal wrote:

In case of failure of a test in macsec autotest,
the buffers were not getting cleaned.
Added appropriate code to clean the buffers.

Fixes: 993ea577a006 ("test/security: add inline MACsec cases")
Cc: sta...@dpdk.org

Signed-off-by: Akhil Goyal <gak...@marvell.com>
---
  app/test/test_security_inline_macsec.c | 70 ++++++++++++++++++--------
  1 file changed, 49 insertions(+), 21 deletions(-)

diff --git a/app/test/test_security_inline_macsec.c 
b/app/test/test_security_inline_macsec.c
index 20670fe5d2..8b57bc51fb 100644
--- a/app/test/test_security_inline_macsec.c
+++ b/app/test/test_security_inline_macsec.c
@@ -837,6 +837,11 @@ test_macsec_event_callback(uint16_t port_id, enum 
rte_eth_event_type type,
        return 0;
  }

  
+#define FREE_PKTS(j, m) {		\

+       while (j--)                     \
+               rte_pktmbuf_free(m[j]); \
+}
+



Changes looks good. However do you want to define a static inline 
function here instead of macro?



  static int
  test_macsec(const struct mcs_test_vector *td[], enum mcs_op op, const struct 
mcs_test_opts *opts)
  {
@@ -878,8 +883,7 @@ test_macsec(const struct mcs_test_vector *td[], enum mcs_op 
op, const struct mcs
                        tx_pkts_burst[j]->ol_flags |= RTE_MBUF_F_TX_MACSEC;
                }
                if (tx_pkts_burst[j] == NULL) {
-                       while (j--)
-                               rte_pktmbuf_free(tx_pkts_burst[j]);
+                       FREE_PKTS(j, tx_pkts_burst);
                        ret = TEST_FAILED;
                        goto out;
                }
@@ -891,8 +895,7 @@ test_macsec(const struct mcs_test_vector *td[], enum mcs_op 
op, const struct mcs
                                        opts->ar_td[k]->secure_pkt.data,
                                        opts->ar_td[k]->secure_pkt.len);
                                if (tx_pkts_burst[j] == NULL) {
-                                       while (j--)
-                                               
rte_pktmbuf_free(tx_pkts_burst[j]);
+                                       FREE_PKTS(j, tx_pkts_burst);
                                        ret = TEST_FAILED;
                                        goto out;
                                }
@@ -919,8 +922,7 @@ test_macsec(const struct mcs_test_vector *td[], enum mcs_op 
op, const struct mcs
                                tx_pkts_burst[j]->ol_flags |= 
RTE_MBUF_F_TX_MACSEC;
                        }
                        if (tx_pkts_burst[j] == NULL) {
-                               while (j--)
-                                       rte_pktmbuf_free(tx_pkts_burst[j]);
+                               FREE_PKTS(j, tx_pkts_burst);
                                ret = TEST_FAILED;
                                goto out;
                        }
@@ -942,7 +944,9 @@ test_macsec(const struct mcs_test_vector *td[], enum mcs_op 
op, const struct mcs
                                id = rte_security_macsec_sa_create(ctx, 
&sa_conf);
                                if (id < 0) {
                                        printf("MACsec SA create failed : 
%d.\n", id);
-                                       return TEST_FAILED;
+                                       FREE_PKTS(j, tx_pkts_burst);
+                                       ret = TEST_FAILED;
+                                       goto out;
                                }
                                rx_sa_id[i][an] = (uint16_t)id;
                        }
@@ -951,6 +955,8 @@ test_macsec(const struct mcs_test_vector *td[], enum mcs_op 
op, const struct mcs
                        id = rte_security_macsec_sc_create(ctx, &sc_conf);
                        if (id < 0) {
                                printf("MACsec SC create failed : %d.\n", id);
+                               FREE_PKTS(j, tx_pkts_burst);
+                               ret = TEST_FAILED;
                                goto out;
                        }
                        rx_sc_id[i] = (uint16_t)id;
@@ -958,19 +964,26 @@ test_macsec(const struct mcs_test_vector *td[], enum 
mcs_op op, const struct mcs
                        /* Create Inline IPsec session. */
                        ret = fill_session_conf(td[i], port_id, opts, 
&sess_conf,
                                        RTE_SECURITY_MACSEC_DIR_RX, 
rx_sc_id[i], tci_off);
-                       if (ret)
-                               return TEST_FAILED;
-
+                       if (ret) {
+                               FREE_PKTS(j, tx_pkts_burst);
+                               ret = TEST_FAILED;
+                               goto out;
+                       }
                        rx_sess[i] = rte_security_session_create(ctx, 
&sess_conf,
                                        sess_pool);
                        if (rx_sess[i] == NULL) {
                                printf("SEC Session init failed.\n");
-                               return TEST_FAILED;
+                               FREE_PKTS(j, tx_pkts_burst);
+                               ret = TEST_FAILED;
+                               goto out;
                        }
                        ret = create_default_flow(td[i], port_id,
                                        RTE_SECURITY_MACSEC_DIR_RX, rx_sess[i]);
-                       if (ret)
+                       if (ret) {
+                               FREE_PKTS(j, tx_pkts_burst);
+                               ret = TEST_FAILED;
                                goto out;
+                       }
                }
                if (op == MCS_ENCAP || op == MCS_ENCAP_DECAP ||
                                op == MCS_AUTH_ONLY || op == MCS_AUTH_VERIFY) {
@@ -983,7 +996,9 @@ test_macsec(const struct mcs_test_vector *td[], enum mcs_op 
op, const struct mcs
                        id = rte_security_macsec_sa_create(ctx, &sa_conf);
                        if (id < 0) {
                                printf("MACsec SA create failed : %d.\n", id);
-                               return TEST_FAILED;
+                               FREE_PKTS(j, tx_pkts_burst);
+                               ret = TEST_FAILED;
+                               goto out;
                        }
                        tx_sa_id[i][0] = (uint16_t)id;
                        tx_sa_id[i][1] = MCS_INVALID_SA;
@@ -997,6 +1012,8 @@ test_macsec(const struct mcs_test_vector *td[], enum 
mcs_op op, const struct mcs
                                id = rte_security_macsec_sa_create(ctx, 
&sa_conf);
                                if (id < 0) {
                                        printf("MACsec rekey SA create failed : 
%d.\n", id);
+                                       FREE_PKTS(j, tx_pkts_burst);
+                                       ret = TEST_FAILED;
                                        goto out;
                                }
                                tx_sa_id[i][1] = (uint16_t)id;
@@ -1006,6 +1023,8 @@ test_macsec(const struct mcs_test_vector *td[], enum 
mcs_op op, const struct mcs
                        id = rte_security_macsec_sc_create(ctx, &sc_conf);
                        if (id < 0) {
                                printf("MACsec SC create failed : %d.\n", id);
+                               FREE_PKTS(j, tx_pkts_burst);
+                               ret = TEST_FAILED;
                                goto out;
                        }
                        tx_sc_id[i] = (uint16_t)id;
@@ -1013,19 +1032,26 @@ test_macsec(const struct mcs_test_vector *td[], enum 
mcs_op op, const struct mcs
                        /* Create Inline IPsec session. */
                        ret = fill_session_conf(td[i], port_id, opts, 
&sess_conf,
                                        RTE_SECURITY_MACSEC_DIR_TX, 
tx_sc_id[i], tci_off);
-                       if (ret)
-                               return TEST_FAILED;
-
+                       if (ret) {
+                               FREE_PKTS(j, tx_pkts_burst);
+                               ret = TEST_FAILED;
+                               goto out;
+                       }
                        tx_sess[i] = rte_security_session_create(ctx, 
&sess_conf,
                                        sess_pool);
                        if (tx_sess[i] == NULL) {
                                printf("SEC Session init failed.\n");
-                               return TEST_FAILED;
+                               FREE_PKTS(j, tx_pkts_burst);
+                               ret = TEST_FAILED;
+                               goto out;
                        }
                        ret = create_default_flow(td[i], port_id,
                                        RTE_SECURITY_MACSEC_DIR_TX, tx_sess[i]);
-                       if (ret)
+                       if (ret) {
+                               FREE_PKTS(j, tx_pkts_burst);
+                               ret = TEST_FAILED;
                                goto out;
+                       }
                }
        }

  
@@ -1042,6 +1068,7 @@ test_macsec(const struct mcs_test_vector *td[], enum mcs_op op, const struct mcs
  
  	rte_pause();
  
+	j = 0;

        /* Receive back packet on loopback interface. */
        do {
                nb_rx += rte_eth_rx_burst(port_id, 0,
@@ -1055,8 +1082,7 @@ test_macsec(const struct mcs_test_vector *td[], enum 
mcs_op op, const struct mcs
        if (nb_rx != nb_sent) {
                printf("\nUnable to RX all %d packets, received(%i)",
                                nb_sent, nb_rx);
-               while (--nb_rx >= 0)
-                       rte_pktmbuf_free(rx_pkts_burst[nb_rx]);
+               FREE_PKTS(nb_rx, rx_pkts_burst);
                ret = TEST_FAILED;
                if (opts->check_sectag_interrupts == 1)
                        ret = TEST_SUCCESS;
@@ -1080,7 +1106,9 @@ test_macsec(const struct mcs_test_vector *td[], enum 
mcs_op op, const struct mcs
                        id = rte_security_macsec_sa_create(ctx, &sa_conf);
                        if (id < 0) {
                                printf("MACsec SA create failed : %d.\n", id);
-                               return TEST_FAILED;
+                               FREE_PKTS(nb_rx, rx_pkts_burst);
+                               ret = TEST_FAILED;
+                               goto out;
                        }
                        tx_sa_id[0][0] = (uint16_t)id;
                        break;






















					Reply via email to