From patchwork Tue Jul 30 12:22:34 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Konstantin Ananyev X-Patchwork-Id: 142759 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id D5F0A456F7; Tue, 30 Jul 2024 15:58:04 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 673D340651; Tue, 30 Jul 2024 15:58:00 +0200 (CEST) Received: from forward500d.mail.yandex.net (forward500d.mail.yandex.net [178.154.239.208]) by mails.dpdk.org (Postfix) with ESMTP id 183EF400D7; Tue, 30 Jul 2024 14:22:51 +0200 (CEST) Received: from mail-nwsmtp-smtp-production-main-42.myt.yp-c.yandex.net (mail-nwsmtp-smtp-production-main-42.myt.yp-c.yandex.net [IPv6:2a02:6b8:c12:3c91:0:640:38da:0]) by forward500d.mail.yandex.net (Yandex) with ESMTPS id 15E7760BB9; Tue, 30 Jul 2024 15:22:50 +0300 (MSK) Received: by mail-nwsmtp-smtp-production-main-42.myt.yp-c.yandex.net (smtp/Yandex) with ESMTPSA id cMUdT3ZoOSw0-hgSVT9XO; Tue, 30 Jul 2024 15:22:49 +0300 X-Yandex-Fwd: 1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yandex.ru; s=mail; t=1722342169; bh=cw0fqz6TbVFnPU+9yXZOFvi0eeY99OP0heFNTg2KyiY=; h=Message-Id:Date:In-Reply-To:Cc:Subject:References:To:From; b=Jp5q5clVnNt47aNAPHZ4UMFoSFMfT06PIC/Vb85op034evNaEex3QuTk+0U1qlZSe 0+oH8XYTa5Sf6LXFFNUtbtbH4FztvmeLwISTs1kHCTYbhzLuPmigkzyjzcQaDllSjZ cUsTe081KkMw8CtIRi5dAsrNKvM24GyevuWwKqOo= Authentication-Results: mail-nwsmtp-smtp-production-main-42.myt.yp-c.yandex.net; dkim=pass header.i=@yandex.ru From: Konstantin Ananyev To: dev@dpdk.org Cc: songx.jiale@intel.com, Konstantin Ananyev , stable@dpdk.org Subject: [PATCH v2 1/2] examples/l3fwd: fix read beyond array bondaries Date: Tue, 30 Jul 2024 13:22:34 +0100 Message-Id: <20240730122235.1084-2-konstantin.v.ananyev@yandex.ru> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20240730122235.1084-1-konstantin.v.ananyev@yandex.ru> References: <20240726194302.920-1-konstantin.v.ananyev@yandex.ru> <20240730122235.1084-1-konstantin.v.ananyev@yandex.ru> MIME-Version: 1.0 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 From: Konstantin Ananyev ASAN report: ERROR: AddressSanitizer: unknown-crash on address 0x7ffffef92e32 at pc 0x00000053d1e9 bp 0x7ffffef92c00 sp 0x7ffffef92bf8 READ of size 16 at 0x7ffffef92e32 thread T0 #0 0x53d1e8 in _mm_loadu_si128 /usr/lib64/gcc/x86_64-suse-linux/11/include/emmintrin.h:703 #1 0x53d1e8 in send_packets_multi ../examples/l3fwd/l3fwd_sse.h:125 #2 0x53d1e8 in acl_send_packets ../examples/l3fwd/l3fwd_acl.c:1048 #3 0x53ec18 in acl_main_loop ../examples/l3fwd/l3fwd_acl.c:1127 #4 0x12151eb in rte_eal_mp_remote_launch ../lib/eal/common/eal_common_launch.c:83 #5 0x5bf2df in main ../examples/l3fwd/main.c:1647 #6 0x7f6d42a0d2bc in __libc_start_main (/lib64/libc.so.6+0x352bc) #7 0x527499 in _start (/home/kananyev/dpdk-l3fwd-acl/x86_64-native-linuxapp-gcc-dbg-b1/examples/dpdk-l3fwd+0x527499) Reason for that is that send_packets_multi() uses 16B loads to access input dst_port[]and might read beyond array boundaries. Right now, it doesn't cause any real issue - junk values are ignored, also inside l3fwd we always allocate dst_port[] array on the stack, so memory beyond it is always available. Anyway, it probably need to be fixed. The patch below simply allocates extra space for dst_port[], so send_packets_multi() will never read beyond its boundaries. Probably a better fix would be to change send_packets_multi() itself to avoid access beyond 'nb_rx' entries. Bugzilla ID: 1502 Fixes: 94c54b4158d5 ("examples/l3fwd: rework exact-match") Cc: stable@dpdk.org Signed-off-by: Konstantin Ananyev Acked-by: Stephen Hemminger --- examples/l3fwd/l3fwd_acl.c | 2 +- examples/l3fwd/l3fwd_altivec.h | 6 +++++- examples/l3fwd/l3fwd_common.h | 7 +++++++ examples/l3fwd/l3fwd_em_hlm.h | 2 +- examples/l3fwd/l3fwd_em_sequential.h | 2 +- examples/l3fwd/l3fwd_fib.c | 2 +- examples/l3fwd/l3fwd_lpm_altivec.h | 2 +- examples/l3fwd/l3fwd_lpm_neon.h | 2 +- examples/l3fwd/l3fwd_lpm_sse.h | 2 +- examples/l3fwd/l3fwd_neon.h | 6 +++++- examples/l3fwd/l3fwd_sse.h | 6 +++++- 11 files changed, 29 insertions(+), 10 deletions(-) diff --git a/examples/l3fwd/l3fwd_acl.c b/examples/l3fwd/l3fwd_acl.c index b635011ef7..baa01e6dde 100644 --- a/examples/l3fwd/l3fwd_acl.c +++ b/examples/l3fwd/l3fwd_acl.c @@ -1056,7 +1056,7 @@ int acl_main_loop(__rte_unused void *dummy) { struct rte_mbuf *pkts_burst[MAX_PKT_BURST]; - uint16_t hops[MAX_PKT_BURST]; + 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; diff --git a/examples/l3fwd/l3fwd_altivec.h b/examples/l3fwd/l3fwd_altivec.h index e45e138e59..b91a6b5587 100644 --- a/examples/l3fwd/l3fwd_altivec.h +++ b/examples/l3fwd/l3fwd_altivec.h @@ -11,6 +11,9 @@ #include "altivec/port_group.h" #include "l3fwd_common.h" +#undef SENDM_PORT_OVERHEAD +#define SENDM_PORT_OVERHEAD(x) ((x) + 2 * FWDSTEP) + /* * Update source and destination MAC addresses in the ethernet header. * Perform RFC1812 checks and updates for IPV4 packets. @@ -117,7 +120,8 @@ process_packet(struct rte_mbuf *pkt, uint16_t *dst_port) */ static __rte_always_inline void send_packets_multi(struct lcore_conf *qconf, struct rte_mbuf **pkts_burst, - uint16_t dst_port[MAX_PKT_BURST], int nb_rx) + uint16_t dst_port[SENDM_PORT_OVERHEAD(MAX_PKT_BURST)], + int nb_rx) { int32_t k; int j = 0; diff --git a/examples/l3fwd/l3fwd_common.h b/examples/l3fwd/l3fwd_common.h index 224b1c08e8..d94e5f1357 100644 --- a/examples/l3fwd/l3fwd_common.h +++ b/examples/l3fwd/l3fwd_common.h @@ -18,6 +18,13 @@ /* Minimum value of IPV4 total length (20B) in network byte order. */ #define IPV4_MIN_LEN_BE (sizeof(struct rte_ipv4_hdr) << 8) +/* + * send_packet_multi() specific number of dest ports + * due to implementation we need to allocate array bigger then + * actual max number of elements in the array. + */ +#define SENDM_PORT_OVERHEAD(x) (x) + /* * From http://www.rfc-editor.org/rfc/rfc1812.txt section 5.2.2: * - The IP version number must be 4. diff --git a/examples/l3fwd/l3fwd_em_hlm.h b/examples/l3fwd/l3fwd_em_hlm.h index 31cda9ddc1..c1d819997a 100644 --- a/examples/l3fwd/l3fwd_em_hlm.h +++ b/examples/l3fwd/l3fwd_em_hlm.h @@ -249,7 +249,7 @@ static inline void l3fwd_em_send_packets(int nb_rx, struct rte_mbuf **pkts_burst, uint16_t portid, struct lcore_conf *qconf) { - uint16_t dst_port[MAX_PKT_BURST]; + uint16_t dst_port[SENDM_PORT_OVERHEAD(MAX_PKT_BURST)]; l3fwd_em_process_packets(nb_rx, pkts_burst, dst_port, portid, qconf, 0); send_packets_multi(qconf, pkts_burst, dst_port, nb_rx); diff --git a/examples/l3fwd/l3fwd_em_sequential.h b/examples/l3fwd/l3fwd_em_sequential.h index 067f23889a..3a40b2e434 100644 --- a/examples/l3fwd/l3fwd_em_sequential.h +++ b/examples/l3fwd/l3fwd_em_sequential.h @@ -79,7 +79,7 @@ l3fwd_em_send_packets(int nb_rx, struct rte_mbuf **pkts_burst, uint16_t portid, struct lcore_conf *qconf) { int32_t i, j; - uint16_t dst_port[MAX_PKT_BURST]; + uint16_t dst_port[SENDM_PORT_OVERHEAD(MAX_PKT_BURST)]; if (nb_rx > 0) { rte_prefetch0(rte_pktmbuf_mtod(pkts_burst[0], diff --git a/examples/l3fwd/l3fwd_fib.c b/examples/l3fwd/l3fwd_fib.c index f38b19af3f..a36330119a 100644 --- a/examples/l3fwd/l3fwd_fib.c +++ b/examples/l3fwd/l3fwd_fib.c @@ -121,7 +121,7 @@ fib_send_packets(int nb_rx, struct rte_mbuf **pkts_burst, { uint32_t ipv4_arr[nb_rx]; uint8_t ipv6_arr[nb_rx][RTE_FIB6_IPV6_ADDR_SIZE]; - uint16_t hops[nb_rx]; + uint16_t hops[SENDM_PORT_OVERHEAD(nb_rx)]; uint64_t hopsv4[nb_rx], hopsv6[nb_rx]; uint8_t type_arr[nb_rx]; uint32_t ipv4_cnt = 0, ipv6_cnt = 0; diff --git a/examples/l3fwd/l3fwd_lpm_altivec.h b/examples/l3fwd/l3fwd_lpm_altivec.h index adb82f1478..91aad5c313 100644 --- a/examples/l3fwd/l3fwd_lpm_altivec.h +++ b/examples/l3fwd/l3fwd_lpm_altivec.h @@ -145,7 +145,7 @@ static inline void l3fwd_lpm_send_packets(int nb_rx, struct rte_mbuf **pkts_burst, uint8_t portid, struct lcore_conf *qconf) { - uint16_t dst_port[MAX_PKT_BURST]; + uint16_t dst_port[SENDM_PORT_OVERHEAD(MAX_PKT_BURST)]; l3fwd_lpm_process_packets(nb_rx, pkts_burst, portid, dst_port, qconf, 0); diff --git a/examples/l3fwd/l3fwd_lpm_neon.h b/examples/l3fwd/l3fwd_lpm_neon.h index 2a68c4c15e..3c1f827424 100644 --- a/examples/l3fwd/l3fwd_lpm_neon.h +++ b/examples/l3fwd/l3fwd_lpm_neon.h @@ -171,7 +171,7 @@ static inline void l3fwd_lpm_send_packets(int nb_rx, struct rte_mbuf **pkts_burst, uint16_t portid, struct lcore_conf *qconf) { - uint16_t dst_port[MAX_PKT_BURST]; + uint16_t dst_port[SENDM_PORT_OVERHEAD(MAX_PKT_BURST)]; l3fwd_lpm_process_packets(nb_rx, pkts_burst, portid, dst_port, qconf, 0); diff --git a/examples/l3fwd/l3fwd_lpm_sse.h b/examples/l3fwd/l3fwd_lpm_sse.h index db15030320..50f1abbd8a 100644 --- a/examples/l3fwd/l3fwd_lpm_sse.h +++ b/examples/l3fwd/l3fwd_lpm_sse.h @@ -129,7 +129,7 @@ static inline void l3fwd_lpm_send_packets(int nb_rx, struct rte_mbuf **pkts_burst, uint16_t portid, struct lcore_conf *qconf) { - uint16_t dst_port[MAX_PKT_BURST]; + uint16_t dst_port[SENDM_PORT_OVERHEAD(MAX_PKT_BURST)]; l3fwd_lpm_process_packets(nb_rx, pkts_burst, portid, dst_port, qconf, 0); diff --git a/examples/l3fwd/l3fwd_neon.h b/examples/l3fwd/l3fwd_neon.h index 40807d5965..bc2bab8265 100644 --- a/examples/l3fwd/l3fwd_neon.h +++ b/examples/l3fwd/l3fwd_neon.h @@ -10,6 +10,9 @@ #include "neon/port_group.h" #include "l3fwd_common.h" +#undef SENDM_PORT_OVERHEAD +#define SENDM_PORT_OVERHEAD(x) ((x) + 2 * FWDSTEP) + /* * Update source and destination MAC addresses in the ethernet header. * Perform RFC1812 checks and updates for IPV4 packets. @@ -92,7 +95,8 @@ process_packet(struct rte_mbuf *pkt, uint16_t *dst_port) */ static __rte_always_inline void send_packets_multi(struct lcore_conf *qconf, struct rte_mbuf **pkts_burst, - uint16_t dst_port[MAX_PKT_BURST], int nb_rx) + uint16_t dst_port[SENDM_PORT_OVERHEAD(MAX_PKT_BURST)], + int nb_rx) { int32_t k; int j = 0; diff --git a/examples/l3fwd/l3fwd_sse.h b/examples/l3fwd/l3fwd_sse.h index 083729cdef..6236b7873c 100644 --- a/examples/l3fwd/l3fwd_sse.h +++ b/examples/l3fwd/l3fwd_sse.h @@ -10,6 +10,9 @@ #include "sse/port_group.h" #include "l3fwd_common.h" +#undef SENDM_PORT_OVERHEAD +#define SENDM_PORT_OVERHEAD(x) ((x) + 2 * FWDSTEP) + /* * Update source and destination MAC addresses in the ethernet header. * Perform RFC1812 checks and updates for IPV4 packets. @@ -91,7 +94,8 @@ process_packet(struct rte_mbuf *pkt, uint16_t *dst_port) */ static __rte_always_inline void send_packets_multi(struct lcore_conf *qconf, struct rte_mbuf **pkts_burst, - uint16_t dst_port[MAX_PKT_BURST], int nb_rx) + uint16_t dst_port[SENDM_PORT_OVERHEAD(MAX_PKT_BURST)], + int nb_rx) { int32_t k; int j = 0; From patchwork Tue Jul 30 12:22:35 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Konstantin Ananyev X-Patchwork-Id: 142760 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id B169E456F7; Tue, 30 Jul 2024 15:58:11 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 401734065B; Tue, 30 Jul 2024 15:58:02 +0200 (CEST) Received: from forward501d.mail.yandex.net (forward501d.mail.yandex.net [178.154.239.209]) by mails.dpdk.org (Postfix) with ESMTP id 373B3400D7 for ; Tue, 30 Jul 2024 14:22:55 +0200 (CEST) Received: from mail-nwsmtp-smtp-production-main-42.myt.yp-c.yandex.net (mail-nwsmtp-smtp-production-main-42.myt.yp-c.yandex.net [IPv6:2a02:6b8:c12:3c91:0:640:38da:0]) by forward501d.mail.yandex.net (Yandex) with ESMTPS id 531B660CC7; Tue, 30 Jul 2024 15:22:54 +0300 (MSK) Received: by mail-nwsmtp-smtp-production-main-42.myt.yp-c.yandex.net (smtp/Yandex) with ESMTPSA id cMUdT3ZoOSw0-FUlyJKAB; Tue, 30 Jul 2024 15:22:53 +0300 X-Yandex-Fwd: 1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yandex.ru; s=mail; t=1722342173; bh=+OJH8UwOW85R8+9lth7oImxW3gZW8isGONTRa+tL5Ro=; h=Message-Id:Date:In-Reply-To:Cc:Subject:References:To:From; b=Ve7wMWpIpY9/EJtoPGnJmgSUvqqeVwjRwZhdTLXXded2CkZ759/IZl0HZSbe8Ovjr XEJn39ZeDM+RDxDHGARzeqz18uSoJWyODGdXu5LXR7l6a6erBdXjCZvnmt0+V4vEec 9rJaZGJmKgruB1MZoLhogv9odfBilN/lyP8bxFns= Authentication-Results: mail-nwsmtp-smtp-production-main-42.myt.yp-c.yandex.net; dkim=pass header.i=@yandex.ru From: Konstantin Ananyev To: dev@dpdk.org Cc: songx.jiale@intel.com, Konstantin Ananyev Subject: [PATCH v2 2/2] examples/l3fwd: fix read beyond array boundaries in ACL mode Date: Tue, 30 Jul 2024 13:22:35 +0100 Message-Id: <20240730122235.1084-3-konstantin.v.ananyev@yandex.ru> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20240730122235.1084-1-konstantin.v.ananyev@yandex.ru> References: <20240726194302.920-1-konstantin.v.ananyev@yandex.ru> <20240730122235.1084-1-konstantin.v.ananyev@yandex.ru> MIME-Version: 1.0 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 From: Konstantin Ananyev 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(). Otherwhise 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..45013ac06e 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 deined 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); } } }