The previous overlap check only tested whether a new fragment's
endpoints fell inside an existing range, missing the case where the
new fragment completely contains an older one.  Use closed-interval
overlap so conflicting fragments are rejected.

Assisted-by: composer-2.5-fast, Cursor
Fixes: 4ea96698f667 ("Userspace datapath: Add fragmentation handling.")
Signed-off-by: Eli Britstein <[email protected]>
---
 lib/ipf.c             |  8 ++---
 tests/ofproto-dpif.at | 73 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+), 4 deletions(-)

diff --git a/lib/ipf.c b/lib/ipf.c
index ff7881235..7da8b5176 100644
--- a/lib/ipf.c
+++ b/lib/ipf.c
@@ -843,16 +843,16 @@ ipf_list_key_lookup(struct ipf *ipf, const struct 
ipf_list_key *key,
     return NULL;
 }
 
+/* Returns true if the new fragment overlaps any existing fragment.  Uses
+ * closed-interval overlap: start_a <= end_b && end_a >= start_b. */
 static bool
 ipf_is_frag_duped(const struct ipf_frag *frag_list, int last_inuse_idx,
                   size_t start_data_byte, size_t end_data_byte)
     /* OVS_REQUIRES(ipf_lock) */
 {
     for (int i = 0; i <= last_inuse_idx; i++) {
-        if ((start_data_byte >= frag_list[i].start_data_byte &&
-            start_data_byte <= frag_list[i].end_data_byte) ||
-            (end_data_byte >= frag_list[i].start_data_byte &&
-             end_data_byte <= frag_list[i].end_data_byte)) {
+        if (start_data_byte <= frag_list[i].end_data_byte &&
+            end_data_byte >= frag_list[i].start_data_byte) {
             return true;
         }
     }
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 1c117f5b4..fbcc5a67e 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -5716,6 +5716,79 @@ AT_CHECK([ovs-appctl dpctl/ipf-get-status -m \
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([ofproto-dpif - fragment handling - reject overlapped fragment])
+OVS_VSWITCHD_START
+add_of_ports br0 1 90
+
+AT_DATA([flows.txt], [dnl
+table=0 in_port=90,ip actions=ct(commit),output:1
+])
+AT_CHECK([ovs-ofctl -O OpenFlow11 replace-flows br0 flows.txt])
+
+dnl The minimum fragment size is clamped to 400 bytes, so both fragments must
+dnl be at least that large to be admitted for reassembly.
+AT_CHECK([ovs-appctl dpctl/ipf-set-min-frag v4 400], [], [dnl
+setting minimum fragment size successful
+])
+
+dnl First admit a fragment carrying bytes 400..799, then reject a new fragment
+dnl covering bytes 0..1199 that fully contains the previously admitted range.
+dnl
+dnl Packet 1 (admitted).  Middle fragment carrying bytes 400..799 (MF set).
+dnl   Ethernet II, Src: 50:54:00:00:00:09, Dst: 50:54:00:00:00:0a
+dnl       Type: IPv4 (0x0800)
+dnl   Internet Protocol Version 4, Src: 10.1.1.1, Dst: 10.1.1.2
+dnl       0100 .... = Version: 4
+dnl       .... 0101 = Header Length: 20 bytes (5)
+dnl       Differentiated Services Field: 0x00 (DSCP: CS0, ECN: Not-ECT)
+dnl       Total Length: 420
+dnl       Identification: 0x0020 (32)
+dnl       001. .... = Flags: 0x1 (More fragments)
+dnl       ...0 0000 0011 0010 = Fragment Offset: 400
+dnl       Time to Live: 64
+dnl       Protocol: UDP (17)
+dnl       Header Checksum: 0x42f3
+dnl   Data (400 bytes)
+eth="50 54 00 00 00 0a 50 54 00 00 00 09 08 00"
+ip1="45 00 01 a4 00 20 20 32 40 11 42 f3"
+addrs="0a 01 01 01 0a 01 01 02"
+data1=$(printf '%0*d' 800 0)
+packet1="${eth}${ip1}${addrs}${data1}"
+
+dnl Packet 2 (rejected as overlap).  First fragment carrying bytes 0..1199
+dnl (MF set), fully containing packet 1's byte range.
+dnl   Ethernet II, Src: 50:54:00:00:00:09, Dst: 50:54:00:00:00:0a
+dnl       Type: IPv4 (0x0800)
+dnl   Internet Protocol Version 4, Src: 10.1.1.1, Dst: 10.1.1.2
+dnl       0100 .... = Version: 4
+dnl       .... 0101 = Header Length: 20 bytes (5)
+dnl       Differentiated Services Field: 0x00 (DSCP: CS0, ECN: Not-ECT)
+dnl       Total Length: 1220
+dnl       Identification: 0x0020 (32)
+dnl       001. .... = Flags: 0x1 (More fragments)
+dnl       ...0 0000 0000 0000 = Fragment Offset: 0
+dnl       Time to Live: 64
+dnl       Protocol: UDP (17)
+dnl       Header Checksum: 0x4005
+dnl   Data (1200 bytes)
+ip2="45 00 04 c4 00 20 20 00 40 11 40 05"
+data2=$(printf '%0*d' 2400 0)
+packet2="${eth}${ip2}${addrs}${data2}"
+
+AT_CHECK([ovs-appctl netdev-dummy/receive p90 "$packet1"])
+AT_CHECK([ovs-appctl netdev-dummy/receive p90 "$packet2"])
+
+AT_CHECK([ovs-appctl dpctl/ipf-get-status -m \
+| grep -E 'num frag:|v4 frags accepted:|v4 frags completed:|v4 frags 
overlapped:'], [], [dnl
+        num frag: 1
+        v4 frags accepted: 1
+        v4 frags completed: 0
+        v4 frags overlapped: 1
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([ofproto-dpif - handling of malformed TCP packets])
 OVS_VSWITCHD_START
 add_of_ports br0 1 90
-- 
2.43.0

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to