[AutoPR- Security] Patch openvswitch for CVE-2026-34956 [MEDIUM]#17252
Open
azurelinux-security wants to merge 1 commit into
Open
[AutoPR- Security] Patch openvswitch for CVE-2026-34956 [MEDIUM]#17252azurelinux-security wants to merge 1 commit into
azurelinux-security wants to merge 1 commit into
Conversation
microsoft-github-policy-service
Bot
added
Packaging
3.0-dev
Contributor
Author
🔒 CVE Patch Review: CVE-2026-34956PR #17252 — [AutoPR- Security] Patch openvswitch for CVE-2026-34956 [MEDIUM] Spec File Validation
Build Verification
🤖 AI Build Log Analysis
🧪 Test Log Analysis
🤖 AI Test Log Analysis
Patch Analysis
Detailed analysisCore fix equivalence:
Tests:
Non-functional differences:
Risk assessment:
Raw diff (upstream vs PR)--- upstream
+++ pr
@@ -1,289 +1,287 @@
-From 8e81545d9d1461758c72fbd08ce1f52e472bca94 Mon Sep 17 00:00:00 2001
-From: Aaron Conole <aconole@redhat.com>
-Date: Tue, 31 Mar 2026 14:39:23 +0200
-Subject: [PATCH] conntrack: Fix replace_substring to handle larger packets.
+diff --git a/SPECS/openvswitch/CVE-2026-34956.patch b/SPECS/openvswitch/CVE-2026-34956.patch
+new file mode 100644
+index 00000000000..4f6baf05c9d
+--- /dev/null
++++ b/SPECS/openvswitch/CVE-2026-34956.patch
+@@ -0,0 +1,279 @@
++From 0034c6f9a642d5462ca073b82db2ea6b6400f8a7 Mon Sep 17 00:00:00 2001
++From: Aaron Conole <aconole@redhat.com>
++Date: Tue, 31 Mar 2026 14:39:23 +0200
++Subject: [PATCH] conntrack: Fix replace_substring to handle larger packets.
++
++There is a buffer size calculation issue in replace_string that can
++result in a heap overflow with a specially crafted FTP packet. This
++is a result of integer truncation when downscaling from size_t into
++uint8_t size. Correct this by setting the types to size_t until the
++underlying memmove to keep the sizes intact.
++
++The total_size, substr_size, and rep_str_size are expected to all be
++sane values for the memmove, and modify_packet also expects this, so
++document that as well. In the case of FTP, those are enforced in
++repl_ftp_v*_addr at the checks for MAX_FTP_V*_NAT_DELTA, and the
++packet data itself should be sanitized by the ovs_strlcpy that runs
++early to extract a string of appropriate length.
++
++Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.")
++Reported-by: Seiji Sakurai <Seiji.Sakurai@outlook.com>
++Signed-off-by: Aaron Conole <aconole@redhat.com>
++Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
++Signed-off-by: Azure Linux Security Servicing Account <azurelinux-security@microsoft.com>
++Upstream-reference: https://github.com/openvswitch/ovs/commit/8e81545d9d1461758c72fbd08ce1f52e472bca94.patch
++---
++ lib/conntrack.c | 9 +-
++ tests/library.at | 4 +
++ tests/test-conntrack.c | 182 +++++++++++++++++++++++++++++++++++++++++
++ 3 files changed, 192 insertions(+), 3 deletions(-)
++
++diff --git a/lib/conntrack.c b/lib/conntrack.c
++index 013709b..10ff0c7 100644
++--- a/lib/conntrack.c
+++++ b/lib/conntrack.c
++@@ -2993,9 +2993,9 @@ expectation_create(struct conntrack *ct, ovs_be16 dst_port,
++ }
++
++ static void
++-replace_substring(char *substr, uint8_t substr_size,
++- uint8_t total_size, char *rep_str,
++- uint8_t rep_str_size)
+++replace_substring(char *substr, size_t substr_size,
+++ size_t total_size, char *rep_str,
+++ size_t rep_str_size)
++ {
++ memmove(substr + rep_str_size, substr + substr_size,
++ total_size - substr_size);
++@@ -3013,6 +3013,9 @@ repl_bytes(char *str, char c1, char c2)
++ }
++ }
++
+++/* Replaces a substring in the packet and rewrites the packet
+++ * size to match. This function assumes the caller has verified
+++ * the lengths to prevent under/over flow. */
++ static void
++ modify_packet(struct dp_packet *pkt, char *pkt_str, size_t size,
++ char *repl_str, size_t repl_size,
++diff --git a/tests/library.at b/tests/library.at
++index 7b4aceb..2079273 100644
++--- a/tests/library.at
+++++ b/tests/library.at
++@@ -306,3 +306,7 @@ AT_CHECK([ovstest test-cooperative-multitasking-nested-yield], [0], [], [dnl
++ cooperative_multitasking|ERR|Nested yield avoided, this is a bug! Enable debug logging for more details.
++ ])
++ AT_CLEANUP
+++
+++AT_SETUP([Conntrack Library - FTP ALG parsing])
+++AT_CHECK([ovstest test-conntrack ftp-alg-large-payload])
+++AT_CLEANUP
++diff --git a/tests/test-conntrack.c b/tests/test-conntrack.c
++index 292b6c0..e59f6cc 100644
++--- a/tests/test-conntrack.c
+++++ b/tests/test-conntrack.c
++@@ -29,6 +29,100 @@
++ static const char payload[] = "50540000000a50540000000908004500001c0000000000"
++ "11a4cd0a0101010a0101020001000200080000";
++
+++/* Build an Ethernet + IPv4 packet. If 'pkt' is NULL a new buffer is
+++ * allocated with 64 bytes of extra headroom so the FTP MTU guard passes.
+++ * The buffer is populated up through the IP header; l4 is set to point
+++ * directly after the IP header. The caller is responsible for filling
+++ * the L4 header and payload that follow. */
+++static struct dp_packet *
+++build_eth_ip_packet(struct dp_packet *pkt, struct eth_addr eth_src,
+++ struct eth_addr eth_dst, ovs_be32 ip_src, ovs_be32 ip_dst,
+++ uint8_t proto, uint16_t payload_alloc)
+++{
+++ struct ip_header *iph;
+++ uint16_t proto_len;
+++
+++ switch (proto) {
+++ case IPPROTO_TCP: proto_len = TCP_HEADER_LEN; break;
+++ case IPPROTO_UDP: proto_len = UDP_HEADER_LEN; break;
+++ case IPPROTO_ICMP: proto_len = ICMP_HEADER_LEN; break;
+++ default: proto_len = 0; break;
+++ }
+++
+++ if (pkt == NULL) {
+++ /* 64-byte extra headroom keeps dp_packet_get_allocated() large enough
+++ * that the FTP V4 MTU guard (orig_used_size + 8 <= allocated) passes
+++ * even when the packet is near its maximum size. */
+++ pkt = dp_packet_new_with_headroom(ETH_HEADER_LEN + IP_HEADER_LEN
+++ + proto_len + payload_alloc, 64);
+++ }
+++
+++ eth_compose(pkt, eth_src, eth_dst, ETH_TYPE_IP,
+++ IP_HEADER_LEN + proto_len + payload_alloc);
+++ iph = dp_packet_l3(pkt);
+++ iph->ip_ihl_ver = IP_IHL_VER(5, 4);
+++ iph->ip_tot_len = htons(IP_HEADER_LEN + proto_len + payload_alloc);
+++ iph->ip_ttl = 64;
+++ iph->ip_proto = proto;
+++ packet_set_ipv4_addr(pkt, &iph->ip_src, ip_src);
+++ packet_set_ipv4_addr(pkt, &iph->ip_dst, ip_dst);
+++ iph->ip_csum = csum(iph, IP_HEADER_LEN);
+++ dp_packet_set_l4(pkt, (char *) iph + IP_HEADER_LEN);
+++ return pkt;
+++}
+++
+++/* Fill the TCP header and optional payload for a packet previously built with
+++ * build_eth_ip_packet(). The 'payload' buffer of 'payload_len' bytes is
+++ * appended after the TCP header if non-NULL. IP total-length, IP checksum,
+++ * and TCP checksum are all updated to reflect the final packet contents. */
+++static struct dp_packet *
+++build_tcp_packet(struct dp_packet *pkt, uint16_t tcp_src, uint16_t tcp_dst,
+++ uint16_t tcp_flags, const char *tcp_payload,
+++ size_t payload_len)
+++{
+++ struct tcp_header *tcph;
+++ struct ip_header *iph;
+++ uint16_t ip_tot_len;
+++ uint32_t tcp_csum;
+++ struct flow flow;
+++
+++ ovs_assert(pkt);
+++ tcph = dp_packet_l4(pkt);
+++ ovs_assert(tcph);
+++
+++ tcph->tcp_src = htons(tcp_src);
+++ tcph->tcp_dst = htons(tcp_dst);
+++ put_16aligned_be32(&tcph->tcp_seq, 0);
+++ put_16aligned_be32(&tcph->tcp_ack, 0);
+++ tcph->tcp_ctl = TCP_CTL(tcp_flags, TCP_HEADER_LEN / 4);
+++ tcph->tcp_winsz = htons(65535);
+++ tcph->tcp_csum = 0;
+++ tcph->tcp_urg = 0;
+++
+++ if (tcp_payload && payload_len > 0) {
+++ /* The caller must have pre-allocated space via build_eth_ip_packet's
+++ * payload_alloc argument. Write directly to avoid a realloc that
+++ * would lose the extra headroom required by the FTP MTU guard. */
+++ memcpy((char *) tcph + TCP_HEADER_LEN, tcp_payload, payload_len);
+++ }
+++
+++ /* Update IP total length and recompute IP checksum. */
+++ iph = dp_packet_l3(pkt);
+++ ip_tot_len = IP_HEADER_LEN + TCP_HEADER_LEN + payload_len;
+++ iph->ip_tot_len = htons(ip_tot_len);
+++ iph->ip_csum = 0;
+++ iph->ip_csum = csum(iph, IP_HEADER_LEN);
+++
+++ /* Compute TCP checksum over pseudo-header + TCP segment. */
+++ tcp_csum = packet_csum_pseudoheader(iph);
+++ tcph->tcp_csum = csum_finish(
+++ csum_continue(tcp_csum, tcph, TCP_HEADER_LEN + payload_len));
+++
+++ /* Set l3/l4 offsets so conntrack can extract a flow key. */
+++ flow_extract(pkt, &flow);
+++ return pkt;
+++}
+++
++ static struct dp_packet_batch *
++ prepare_packets(size_t n, bool change, unsigned tid, ovs_be16 *dl_type)
++ {
++@@ -251,6 +345,87 @@ test_pcap(struct ovs_cmdl_context *ctx)
++ conntrack_destroy(ct);
++ ovs_pcap_close(pcap);
++ }
+++
-There is a buffer size calculation issue in replace_string that can
-result in a heap overflow with a specially crafted FTP packet. This
-is a result of integer truncation when downscaling from size_t into
-uint8_t size. Correct this by setting the types to size_t until the
-underlying memmove to keep the sizes intact.
+++/* ALG related testing. */
+++
+++/* FTP IPv4 PORT payload for testing. */
+++#define FTP_PORT_CMD_STR "PORT 192,168,123,2,113,42\r\n"
+++#define FTP_CMD_PAD 234
+++#define FTP_PAYLOAD_LEN (sizeof FTP_PORT_CMD_STR - 1 + FTP_CMD_PAD)
+++
+++/* Test modify_packet wrapping.
+++ *
+++ * The test builds a minimal FTP control-channel exchange:
+++ * 1. A TCP SYN that creates a conntrack entry with helper=ftp and SNAT.
+++ * 2. A PSH|ACK carrying "PORT 192,168,123,2,113,42\r\n" padded to exactly
+++ * 261 bytes of TCP payload, which makes total_size == 256.
+++ *
+++ * After the PORT packet is processed the address field in the payload must
+++ * read "192,168,1,1" (the SNAT address with dots replaced by commas). */
+++static void
+++test_ftp_alg_large_payload(struct ovs_cmdl_context *ctx OVS_UNUSED)
+++{
+++ /* Packet endpoints. */
+++ struct eth_addr eth_src = ETH_ADDR_C(00, 01, 02, 03, 04, 05);
+++ struct eth_addr eth_dst = ETH_ADDR_C(00, 06, 07, 08, 09, 0a);
+++ ovs_be32 ip_src = inet_addr("192.168.123.2"); /* FTP client. */
+++ ovs_be32 ip_dst = inet_addr("192.168.1.1"); /* FTP server / SNAT addr. */
+++ uint16_t sport = 12345;
+++ uint16_t dport = 21; /* FTP control port. */
+++
+++ /* SNAT: rewrite client address to 192.168.1.1 in PORT commands. */
+++ struct nat_action_info_t nat_info;
+++ memset(&nat_info, 0, sizeof nat_info);
+++ nat_info.nat_action = NAT_ACTION_SRC;
+++ nat_info.min_addr.ipv4 = ip_dst;
+++ nat_info.max_addr.ipv4 = ip_dst;
+++
+++ ct = conntrack_init();
+++ conntrack_set_tcp_seq_chk(ct, false);
+++
+++ long long now = time_msec();
+++
+++ struct dp_packet *syn = build_eth_ip_packet(NULL, eth_src, eth_dst,
+++ ip_src, ip_dst,
+++ IPPROTO_TCP, 0);
+++ build_tcp_packet(syn, sport, dport, TCP_SYN, NULL, 0);
+++
+++ struct dp_packet_batch syn_batch;
+++ dp_packet_batch_init_packet(&syn_batch, syn);
+++ conntrack_execute(ct, &syn_batch, htons(ETH_TYPE_IP), false, true, 0,
+++ NULL, NULL, "ftp", &nat_info, now, 0);
+++ dp_packet_delete_batch(&syn_batch, true);
+++
+++ /* We get to skip some of the processing because the conntrack execute
+++ * above will create the required conntrack entries. */
+++
+++ /* Build the large payload: PORT command followed by padding spaces
+++ * and a final "\r\n" to reach exactly FTP_PAYLOAD_LEN bytes. The
+++ * FTP parser only looks at the first LARGEST_FTP_MSG_OF_INTEREST (128)
+++ * bytes, so the trailing spaces do not interfere with parsing. */
+++ char ftp_payload[FTP_PAYLOAD_LEN];
+++ memcpy(ftp_payload, FTP_PORT_CMD_STR, sizeof FTP_PORT_CMD_STR - 1);
+++ memset(ftp_payload + sizeof FTP_PORT_CMD_STR - 1, ' ', FTP_CMD_PAD);
+++
+++ struct dp_packet *port_pkt =
+++ build_eth_ip_packet(NULL, eth_src, eth_dst, ip_src, ip_dst,
+++ IPPROTO_TCP, FTP_PAYLOAD_LEN);
+++ build_tcp_packet(port_pkt, sport, dport, TCP_PSH | TCP_ACK,
+++ ftp_payload, FTP_PAYLOAD_LEN);
+++
+++ struct dp_packet_batch port_batch;
+++ dp_packet_batch_init_packet(&port_batch, port_pkt);
+++ conntrack_execute(ct, &port_batch, htons(ETH_TYPE_IP), false, true, 0,
+++ NULL, NULL, "ftp", &nat_info, now, 0);
+++
+++ struct tcp_header *th = dp_packet_l4(port_pkt);
+++ size_t tcp_hdr_len = TCP_OFFSET(th->tcp_ctl) * 4;
+++ const char *ftp_start = (const char *) th + tcp_hdr_len;
+++ ovs_assert(!strncmp(ftp_start, "PORT 192,168,1,1,", 17));
+++ dp_packet_delete_batch(&port_batch, true);
+++ conntrack_destroy(ct);
+++}
+++
++
-The total_size, substr_size, and rep_str_size are expected to all be
-sane values for the memmove, and modify_packet also expects this, so
-document that as well. In the case of FTP, those are enforced in
-repl_ftp_v*_addr at the checks for MAX_FTP_V*_NAT_DELTA, and the
-packet data itself should be sanitized by the ovs_strlcpy that runs
-early to extract a string of appropriate length.
-
-Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.")
-Reported-by: Seiji Sakurai <Seiji.Sakurai@outlook.com>
-Signed-off-by: Aaron Conole <aconole@redhat.com>
-Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
----
- AUTHORS.rst | 1 +
- lib/conntrack.c | 9 +-
- tests/library.at | 4 +
- tests/test-conntrack.c | 182 +++++++++++++++++++++++++++++++++++++++++
- 4 files changed, 193 insertions(+), 3 deletions(-)
-
-diff --git a/AUTHORS.rst b/AUTHORS.rst
-index fe4064ca71d..25f71fc0af3 100644
---- a/AUTHORS.rst
-+++ b/AUTHORS.rst
-@@ -728,6 +728,7 @@ Scott Hendricks
- Sean Brady sbrady@gtfservices.com
- Sebastian Andrzej Siewior sebastian@breakpoint.cc
- Sébastien RICCIO sr@swisscenter.com
-+Seiji Sakurai Seiji.Sakurai@outlook.com
- Shweta Seth shwseth@cisco.com
- Simon Jouet simon.jouet@gmail.com
- Spiro Kourtessis spiro@vmware.com
-diff --git a/lib/conntrack.c b/lib/conntrack.c
-index 483da5cc934..80d2e424f42 100644
---- a/lib/conntrack.c
-+++ b/lib/conntrack.c
-@@ -3025,9 +3025,9 @@ expectation_create(struct conntrack *ct, ovs_be16 dst_port,
- }
-
- static void
--replace_substring(char *substr, uint8_t substr_size,
-- uint8_t total_size, char *rep_str,
-- uint8_t rep_str_size)
-+replace_substring(char *substr, size_t substr_size,
-+ size_t total_size, char *rep_str,
-+ size_t rep_str_size)
- {
- memmove(substr + rep_str_size, substr + substr_size,
- total_size - substr_size);
-@@ -3045,6 +3045,9 @@ repl_bytes(char *str, char c1, char c2)
- }
- }
-
-+/* Replaces a substring in the packet and rewrites the packet
-+ * size to match. This function assumes the caller has verified
-+ * the lengths to prevent under/over flow. */
- static void
- modify_packet(struct dp_packet *pkt, char *pkt_str, size_t size,
- char *repl_str, size_t repl_size,
-diff --git a/tests/library.at b/tests/library.at
-index d962e1b3fd2..b1843298d57 100644
---- a/tests/library.at
-+++ b/tests/library.at
-@@ -308,3 +308,7 @@ AT_CHECK([ovstest test-cooperative-multitasking-nested-yield], [0], [], [dnl
- cooperative_multitasking|ERR|Nested yield avoided, this is a bug! Enable debug logging for more details.
- ])
- AT_CLEANUP
-+
-+AT_SETUP([Conntrack Library - FTP ALG parsing])
-+AT_CHECK([ovstest test-conntrack ftp-alg-large-payload])
-+AT_CLEANUP
-diff --git a/tests/test-conntrack.c b/tests/test-conntrack.c
-index 292b6c048b8..e59f6cc790e 100644
---- a/tests/test-conntrack.c
-+++ b/tests/test-conntrack.c
-@@ -29,6 +29,100 @@
- static const char payload[] = "50540000000a50540000000908004500001c0000000000"
- "11a4cd0a0101010a0101020001000200080000";
-
-+/* Build an Ethernet + IPv4 packet. If 'pkt' is NULL a new buffer is
-+ * allocated with 64 bytes of extra headroom so the FTP MTU guard passes.
-+ * The buffer is populated up through the IP header; l4 is set to point
-+ * directly after the IP header. The caller is responsible for filling
-+ * the L4 header and payload that follow. */
-+static struct dp_packet *
-+build_eth_ip_packet(struct dp_packet *pkt, struct eth_addr eth_src,
-+ struct eth_addr eth_dst, ovs_be32 ip_src, ovs_be32 ip_dst,
-+ uint8_t proto, uint16_t payload_alloc)
-+{
-+ struct ip_header *iph;
-+ uint16_t proto_len;
-+
-+ switch (proto) {
-+ case IPPROTO_TCP: proto_len = TCP_HEADER_LEN; break;
-+ case IPPROTO_UDP: proto_len = UDP_HEADER_LEN; break;
-+ case IPPROTO_ICMP: proto_len = ICMP_HEADER_LEN; break;
-+ default: proto_len = 0; break;
-+ }
-+
-+ if (pkt == NULL) {
-+ /* 64-byte extra headroom keeps dp_packet_get_allocated() large enough
-+ * that the FTP V4 MTU guard (orig_used_size + 8 <= allocated) passes
-+ * even when the packet is near its maximum size. */
-+ pkt = dp_packet_new_with_headroom(ETH_HEADER_LEN + IP_HEADER_LEN
-+ + proto_len + payload_alloc, 64);
-+ }
-+
-+ eth_compose(pkt, eth_src, eth_dst, ETH_TYPE_IP,
-+ IP_HEADER_LEN + proto_len + payload_alloc);
-+ iph = dp_packet_l3(pkt);
-+ iph->ip_ihl_ver = IP_IHL_VER(5, 4);
-+ iph->ip_tot_len = htons(IP_HEADER_LEN + proto_len + payload_alloc);
-+ iph->ip_ttl = 64;
-+ iph->ip_proto = proto;
-+ packet_set_ipv4_addr(pkt, &iph->ip_src, ip_src);
-+ packet_set_ipv4_addr(pkt, &iph->ip_dst, ip_dst);
-+ iph->ip_csum = csum(iph, IP_HEADER_LEN);
-+ dp_packet_set_l4(pkt, (char *) iph + IP_HEADER_LEN);
-+ return pkt;
-+}
-+
-+/* Fill the TCP header and optional payload for a packet previously built with
-+ * build_eth_ip_packet(). The 'payload' buffer of 'payload_len' bytes is
-+ * appended after the TCP header if non-NULL. IP total-length, IP checksum,
-+ * and TCP checksum are all updated to reflect the final packet contents. */
-+static struct dp_packet *
-+build_tcp_packet(struct dp_packet *pkt, uint16_t tcp_src, uint16_t tcp_dst,
-+ uint16_t tcp_flags, const char *tcp_payload,
-+ size_t payload_len)
-+{
-+ struct tcp_header *tcph;
-+ struct ip_header *iph;
-+ uint16_t ip_tot_len;
-+ uint32_t tcp_csum;
-+ struct flow flow;
-+
-+ ovs_assert(pkt);
-+ tcph = dp_packet_l4(pkt);
-+ ovs_assert(tcph);
-+
-+ tcph->tcp_src = htons(tcp_src);
-+ tcph->tcp_dst = htons(tcp_dst);
-+ put_16aligned_be32(&tcph->tcp_seq, 0);
-+ put_16aligned_be32(&tcph->tcp_ack, 0);
-+ tcph->tcp_ctl = TCP_CTL(tcp_flags, TCP_HEADER_LEN / 4);
-+ tcph->tcp_winsz = htons(65535);
-+ tcph->tcp_csum = 0;
-+ tcph->tcp_urg = 0;
-+
-+ if (tcp_payload && payload_len > 0) {
-+ /* The caller must have pre-allocated space via build_eth_ip_packet's
-+ * payload_alloc argument. Write directly to avoid a realloc that
-+ * would lose the extra headroom required by the FTP MTU guard. */
-+ memcpy((char *) tcph + TCP_HEADER_LEN, tcp_payload, payload_len);
-+ }
-+
-+ /* Update IP total length and recompute IP checksum. */
-+ iph = dp_packet_l3(pkt);
-+ ip_tot_len = IP_HEADER_LEN + TCP_HEADER_LEN + payload_len;
-+ iph->ip_tot_len = htons(ip_tot_len);
-+ iph->ip_csum = 0;
-+ iph->ip_csum = csum(iph, IP_HEADER_LEN);
-+
-+ /* Compute TCP checksum over pseudo-header + TCP segment. */
-+ tcp_csum = packet_csum_pseudoheader(iph);
-+ tcph->tcp_csum = csum_finish(
-+ csum_continue(tcp_csum, tcph, TCP_HEADER_LEN + payload_len));
-+
-+ /* Set l3/l4 offsets so conntrack can extract a flow key. */
-+ flow_extract(pkt, &flow);
-+ return pkt;
-+}
-+
- static struct dp_packet_batch *
- prepare_packets(size_t n, bool change, unsigned tid, ovs_be16 *dl_type)
- {
-@@ -251,6 +345,87 @@ test_pcap(struct ovs_cmdl_context *ctx)
- conntrack_destroy(ct);
- ovs_pcap_close(pcap);
- }
-+ -
-+/* ALG related testing. */
-+
-+/* FTP IPv4 PORT payload for testing. */
-+#define FTP_PORT_CMD_STR "PORT 192,168,123,2,113,42\r\n"
-+#define FTP_CMD_PAD 234
-+#define FTP_PAYLOAD_LEN (sizeof FTP_PORT_CMD_STR - 1 + FTP_CMD_PAD)
-+
-+/* Test modify_packet wrapping.
-+ *
-+ * The test builds a minimal FTP control-channel exchange:
-+ * 1. A TCP SYN that creates a conntrack entry with helper=ftp and SNAT.
-+ * 2. A PSH|ACK carrying "PORT 192,168,123,2,113,42\r\n" padded to exactly
-+ * 261 bytes of TCP payload, which makes total_size == 256.
-+ *
-+ * After the PORT packet is processed the address field in the payload must
-+ * read "192,168,1,1" (the SNAT address with dots replaced by commas). */
-+static void
-+test_ftp_alg_large_payload(struct ovs_cmdl_context *ctx OVS_UNUSED)
-+{
-+ /* Packet endpoints. */
-+ struct eth_addr eth_src = ETH_ADDR_C(00, 01, 02, 03, 04, 05);
-+ struct eth_addr eth_dst = ETH_ADDR_C(00, 06, 07, 08, 09, 0a);
-+ ovs_be32 ip_src = inet_addr("192.168.123.2"); /* FTP client. */
-+ ovs_be32 ip_dst = inet_addr("192.168.1.1"); /* FTP server / SNAT addr. */
-+ uint16_t sport = 12345;
-+ uint16_t dport = 21; /* FTP control port. */
-+
-+ /* SNAT: rewrite client address to 192.168.1.1 in PORT commands. */
-+ struct nat_action_info_t nat_info;
-+ memset(&nat_info, 0, sizeof nat_info);
-+ nat_info.nat_action = NAT_ACTION_SRC;
-+ nat_info.min_addr.ipv4 = ip_dst;
-+ nat_info.max_addr.ipv4 = ip_dst;
-+
-+ ct = conntrack_init();
-+ conntrack_set_tcp_seq_chk(ct, false);
-+
-+ long long now = time_msec();
-+
-+ struct dp_packet *syn = build_eth_ip_packet(NULL, eth_src, eth_dst,
-+ ip_src, ip_dst,
-+ IPPROTO_TCP, 0);
-+ build_tcp_packet(syn, sport, dport, TCP_SYN, NULL, 0);
-+
-+ struct dp_packet_batch syn_batch;
-+ dp_packet_batch_init_packet(&syn_batch, syn);
-+ conntrack_execute(ct, &syn_batch, htons(ETH_TYPE_IP), false, true, 0,
-+ NULL, NULL, "ftp", &nat_info, now, 0);
-+ dp_packet_delete_batch(&syn_batch, true);
-+
-+ /* We get to skip some of the processing because the conntrack execute
-+ * above will create the required conntrack entries. */
-+
-+ /* Build the large payload: PORT command followed by padding spaces
-+ * and a final "\r\n" to reach exactly FTP_PAYLOAD_LEN bytes. The
-+ * FTP parser only looks at the first LARGEST_FTP_MSG_OF_INTEREST (128)
-+ * bytes, so the trailing spaces do not interfere with parsing. */
-+ char ftp_payload[FTP_PAYLOAD_LEN];
-+ memcpy(ftp_payload, FTP_PORT_CMD_STR, sizeof FTP_PORT_CMD_STR - 1);
-+ memset(ftp_payload + sizeof FTP_PORT_CMD_STR - 1, ' ', FTP_CMD_PAD);
-+
-+ struct dp_packet *port_pkt =
-+ build_eth_ip_packet(NULL, eth_src, eth_dst, ip_src, ip_dst,
-+ IPPROTO_TCP, FTP_PAYLOAD_LEN);
-+ build_tcp_packet(port_pkt, sport, dport, TCP_PSH | TCP_ACK,
-+ ftp_payload, FTP_PAYLOAD_LEN);
-+
-+ struct dp_packet_batch port_batch;
-+ dp_packet_batch_init_packet(&port_batch, port_pkt);
-+ conntrack_execute(ct, &port_batch, htons(ETH_TYPE_IP), false, true, 0,
-+ NULL, NULL, "ftp", &nat_info, now, 0);
-+
-+ struct tcp_header *th = dp_packet_l4(port_pkt);
-+ size_t tcp_hdr_len = TCP_OFFSET(th->tcp_ctl) * 4;
-+ const char *ftp_start = (const char *) th + tcp_hdr_len;
-+ ovs_assert(!strncmp(ftp_start, "PORT 192,168,1,1,", 17));
-+ dp_packet_delete_batch(&port_batch, true);
-+ conntrack_destroy(ct);
-+}
-+
- -
- static const struct ovs_cmdl_command commands[] = {
- /* Connection tracker tests. */
-@@ -264,6 +439,13 @@ static const struct ovs_cmdl_command commands[] = {
- * 'batch_size' (1 by default) per call, with the commit flag set.
- * Prints the ct_state of each packet. */
- {"pcap", "file [batch_size]", 1, 2, test_pcap, OVS_RO},
-+ /* Verifies that the FTP ALG replace_substring function correctly handles
-+ * a packet whose payload puts total_size at exactly 256 bytes. The
-+ * original uint8_t parameter type truncated 256 to 0, leading to a
-+ * near-SIZE_MAX memmove (heap overflow). The test confirms the address
-+ * is rewritten to the SNAT target rather than causing a crash. */
-+ {"ftp-alg-large-payload", "", 0, 0,
-+ test_ftp_alg_large_payload, OVS_RO},
-
- {NULL, NULL, 0, 0, NULL, OVS_RO},
- };
++ static const struct ovs_cmdl_command commands[] = {
++ /* Connection tracker tests. */
++@@ -264,6 +439,13 @@ static const struct ovs_cmdl_command commands[] = {
++ * 'batch_size' (1 by default) per call, with the commit flag set.
++ * Prints the ct_state of each packet. */
++ {"pcap", "file [batch_size]", 1, 2, test_pcap, OVS_RO},
+++ /* Verifies that the FTP ALG replace_substring function correctly handles
+++ * a packet whose payload puts total_size at exactly 256 bytes. The
+++ * original uint8_t parameter type truncated 256 to 0, leading to a
+++ * near-SIZE_MAX memmove (heap overflow). The test confirms the address
+++ * is rewritten to the SNAT target rather than causing a crash. */
+++ {"ftp-alg-large-payload", "", 0, 0,
+++ test_ftp_alg_large_payload, OVS_RO},
++
++ {NULL, NULL, 0, 0, NULL, OVS_RO},
++ };
++--
++2.45.4
++
Verdict❌ CHANGES REQUESTED — Please address the issues flagged above. |
Kanishk-Bansal
approved these changes
May 15, 2026
Contributor
Kanishk-Bansal
left a comment
Kanishk-Bansal
left a comment
There was a problem hiding this comment.
Patch Analysis (the patch matches upstream, except the credit note author file)
- Buddy Build
- patch applied during the build (check
rpm.log) - patch include an upstream reference
- PR has security tag
Kanishk-Bansal
added
the
ready-for-stable-review
kgodara912
requested changes
May 18, 2026
Contributor
kgodara912
left a comment
kgodara912
left a comment
There was a problem hiding this comment.
Could you please check the test failures? Were they preexisting or caused by this patch? If they are known to be flaky, we can try to fix them in a separate PR.
level=debug msg="ERROR: 2609 tests were run,
level=debug msg="2 failed unexpectedly.
level=debug msg="4 tests were skipped.
level=debug msg="## -------------------------- ##
level=debug msg="## testsuite.log was created. ##
## -------------------------- ##
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Auto Patch openvswitch for CVE-2026-34956.
Autosec pipeline run -> https://dev.azure.com/mariner-org/mariner/_build/results?buildId=1118727&view=results
Merge Checklist
All boxes should be checked before merging the PR (just tick any boxes which don't apply to this PR)
*-staticsubpackages, etc.) have had theirReleasetag incremented../cgmanifest.json,./toolkit/scripts/toolchain/cgmanifest.json,.github/workflows/cgmanifest.json)./LICENSES-AND-NOTICES/SPECS/data/licenses.json,./LICENSES-AND-NOTICES/SPECS/LICENSES-MAP.md,./LICENSES-AND-NOTICES/SPECS/LICENSE-EXCEPTIONS.PHOTON)*.signatures.jsonfilessudo make go-tidy-allandsudo make go-test-coveragepassSummary
What does the PR accomplish, why was it needed?
Change Log
Does this affect the toolchain?
YES/NO
Associated issues
Links to CVEs
Test Methodology