From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 8C6DD45CA2; Thu, 7 Nov 2024 19:00:46 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 6A6114329E; Thu, 7 Nov 2024 19:00:37 +0100 (CET) Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) by mails.dpdk.org (Postfix) with ESMTP id 880434329E for ; Thu, 7 Nov 2024 19:00:36 +0100 (CET) Received: from mail.maildlp.com (unknown [172.18.186.31]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4XkqZQ2gggz6K5bx; Fri, 8 Nov 2024 01:58:54 +0800 (CST) Received: from frapeml500007.china.huawei.com (unknown [7.182.85.172]) by mail.maildlp.com (Postfix) with ESMTPS id 20EDB1404F9; Fri, 8 Nov 2024 02:00:35 +0800 (CST) Received: from localhost.localdomain (10.220.239.45) by frapeml500007.china.huawei.com (7.182.85.172) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Thu, 7 Nov 2024 19:00:34 +0100 From: Konstantin Ananyev To: CC: , Subject: [PATCH v3 2/2] examples/l3fwd: fix read beyond array boundaries in ACL mode Date: Thu, 7 Nov 2024 13:50:52 -0500 Message-ID: <20241107185052.64374-3-konstantin.ananyev@huawei.com> X-Mailer: git-send-email 2.35.3 In-Reply-To: <20241107185052.64374-1-konstantin.ananyev@huawei.com> References: <20240730122235.1084-1-konstantin.v.ananyev@yandex.ru> <20241107185052.64374-1-konstantin.ananyev@huawei.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-Originating-IP: [10.220.239.45] X-ClientProxiedBy: frapeml500002.china.huawei.com (7.182.85.205) To frapeml500007.china.huawei.com (7.182.85.172) X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org With commit: ACL mode now can use send_packets_multi(). What I missed with that changes: send_packets_multi() can't deal properly with input dst_port[i] == BAD_PORT (though it can set it itself), as it uses dst_port[i] values to read L2 addresses for the port and assumes dst_port[] to contain valid only values. To fix that just add a check that all dst_port[] entries are valid before calling : send_packets_multi(). Otherwise use send_packets_single(). An alternative, and probably more logical approach would be to re-arrange send_packets_multi() so that it updates L2 packet headers at the very last state - when dst_port[] are finialized. But that would affect all other modes, but that would affect all other modes and will require much more code changes and testing. Bugzilla ID: 1502 Fixes: aa7c6077c19b ("examples/l3fwd: avoid packets reorder in ACL mode") Reported-by: Song Jiale Signed-off-by: Konstantin Ananyev Acked-by: Stephen Hemminger --- examples/l3fwd/l3fwd_acl.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/examples/l3fwd/l3fwd_acl.c b/examples/l3fwd/l3fwd_acl.c index baa01e6dde..c30ba07c1a 100644 --- a/examples/l3fwd/l3fwd_acl.c +++ b/examples/l3fwd/l3fwd_acl.c @@ -993,11 +993,15 @@ dump_denied_pkt(const struct rte_mbuf *pkt, uint32_t res) #endif } -static inline void +/* + * run packets through ACL classify. + * returns number of packets to be dropped (hops[i] == BAD_PORT) + */ +static inline uint32_t acl_process_pkts(struct rte_mbuf *pkts[MAX_PKT_BURST], uint16_t hops[MAX_PKT_BURST], uint32_t num, int32_t socketid) { - uint32_t i, n4, n6, res; + uint32_t i, k, n4, n6, res; struct acl_search_t acl_search; /* split packets burst depending on packet type (IPv4/IPv6) */ @@ -1020,6 +1024,7 @@ acl_process_pkts(struct rte_mbuf *pkts[MAX_PKT_BURST], /* combine lookup results back, into one array of next hops */ n4 = 0; n6 = 0; + k = 0; for (i = 0; i != num; i++) { switch (acl_search.types[i]) { case TYPE_IPV4: @@ -1034,21 +1039,33 @@ acl_process_pkts(struct rte_mbuf *pkts[MAX_PKT_BURST], if (likely((res & ACL_DENY_SIGNATURE) == 0 && res != 0)) hops[i] = res - FWD_PORT_SHIFT; else { + /* bad or denied by ACL rule packets */ hops[i] = BAD_PORT; dump_denied_pkt(pkts[i], res); + k++; } } + + return k; } +/* + * send_packets_multi() can't deal properly with hops[i] == BAD_PORT + * (it assumes input hops[] contain only valid port numbers), + * so it is ok to use it only when there are no denied packets. + */ static inline void acl_send_packets(struct lcore_conf *qconf, struct rte_mbuf *pkts[], - uint16_t hops[], uint32_t num) + uint16_t hops[], uint32_t num, uint32_t nb_drop) { #if defined ACL_SEND_MULTI - send_packets_multi(qconf, pkts, hops, num); + if (nb_drop == 0) + send_packets_multi(qconf, pkts, hops, num); + else #else - send_packets_single(qconf, pkts, hops, num); + RTE_SET_USED(nb_drop); #endif + send_packets_single(qconf, pkts, hops, num); } /* main processing loop */ @@ -1059,7 +1076,7 @@ acl_main_loop(__rte_unused void *dummy) uint16_t hops[SENDM_PORT_OVERHEAD(MAX_PKT_BURST)]; unsigned int lcore_id; uint64_t prev_tsc, diff_tsc, cur_tsc; - int i, nb_rx; + int i, nb_drop, nb_rx; uint16_t portid; uint16_t queueid; struct lcore_conf *qconf; @@ -1122,10 +1139,10 @@ acl_main_loop(__rte_unused void *dummy) pkts_burst, MAX_PKT_BURST); if (nb_rx > 0) { - acl_process_pkts(pkts_burst, hops, nb_rx, - socketid); + nb_drop = acl_process_pkts(pkts_burst, hops, + nb_rx, socketid); acl_send_packets(qconf, pkts_burst, hops, - nb_rx); + nb_rx, nb_drop); } } } -- 2.35.3