IP reassembly must not accept data past an MF=0 fragment. Track
last fragments, reject ranges beyond them on insert, and require the
sorted rightmost fragment to be MF=0 before completing a list.
The accompanying test is added to tests/ofproto-dpif.at (using
netdev-dummy/receive with variable-built packets and precomputed IPv4
header checksums) rather than the system-traffic suite.
Assisted-by: composer-2.5-fast, Cursor
Fixes: 4ea96698f667 ("Userspace datapath: Add fragmentation handling.")
Signed-off-by: Eli Britstein <[email protected]>
---
lib/dpctl.c | 4 ++
lib/dpif-provider.h | 1 +
lib/ipf.c | 50 +++++++++++++++++++--
lib/ipf.h | 1 +
tests/ofproto-dpif.at | 74 ++++++++++++++++++++++++++++++++
tests/system-userspace-macros.at | 8 ++++
6 files changed, 135 insertions(+), 3 deletions(-)
diff --git a/lib/dpctl.c b/lib/dpctl.c
index 76b219e2c..389ee0a42 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -2631,6 +2631,8 @@ dpctl_ct_ipf_get_status(int argc, const char *argv[],
dpif_ipf_status.v4.nfrag_overlap);
dpctl_print(dpctl_p, " v4 frags duplicate: %"PRIu64"\n",
dpif_ipf_status.v4.nfrag_duplicate);
+ dpctl_print(dpctl_p, " v4 frags beyond last: %"PRIu64"\n",
+ dpif_ipf_status.v4.nfrag_beyond_last);
dpctl_print(dpctl_p, " v4 frags purged: %"PRIu64"\n",
dpif_ipf_status.v4.nfrag_purged);
@@ -2650,6 +2652,8 @@ dpctl_ct_ipf_get_status(int argc, const char *argv[],
dpif_ipf_status.v6.nfrag_overlap);
dpctl_print(dpctl_p, " v6 frags duplicate: %"PRIu64"\n",
dpif_ipf_status.v6.nfrag_duplicate);
+ dpctl_print(dpctl_p, " v6 frags beyond last: %"PRIu64"\n",
+ dpif_ipf_status.v6.nfrag_beyond_last);
dpctl_print(dpctl_p, " v6 frags purged: %"PRIu64"\n",
dpif_ipf_status.v6.nfrag_purged);
} else {
diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index 7c9cd003e..268379fe3 100644
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -132,6 +132,7 @@ struct dpif_ipf_proto_status {
uint64_t nfrag_too_large;
uint64_t nfrag_overlap;
uint64_t nfrag_duplicate;
+ uint64_t nfrag_beyond_last;
uint64_t nfrag_purged;
unsigned int min_frag_size;
bool enabled;
diff --git a/lib/ipf.c b/lib/ipf.c
index 7bbfeeda3..146c5bf60 100644
--- a/lib/ipf.c
+++ b/lib/ipf.c
@@ -85,6 +85,7 @@ enum ipf_counter_type {
IPF_NFRAGS_TOO_LARGE,
IPF_NFRAGS_OVERLAP,
IPF_NFRAGS_DUPLICATE,
+ IPF_NFRAGS_BEYOND_LAST,
IPF_NFRAGS_PURGED,
IPF_NFRAGS_NUM_CNTS,
};
@@ -99,6 +100,7 @@ struct ipf_frag {
struct dp_packet *pkt;
uint16_t start_data_byte;
uint16_t end_data_byte;
+ bool last_frag; /* True if this was the MF=0 fragment. */
};
/* The key for a collection of fragments potentially making up an unfragmented
@@ -398,8 +400,14 @@ ipf_list_complete(const struct ipf_list *ipf_list)
!= ipf_list->frag_list[i].start_data_byte) {
return false;
}
+
+ /* Only the final fragment may have MF=0; data past it is invalid. */
+ if (ipf_list->frag_list[i - 1].last_frag) {
+ return false;
+ }
}
- return true;
+
+ return ipf_list->frag_list[ipf_list->last_inuse_idx].last_frag;
}
/* Runs O(n) for a sorted or almost sorted list. */
@@ -895,6 +903,30 @@ ipf_is_frag_overlap(const struct ipf_frag *frag_list, int
last_inuse_idx,
return false;
}
+/* Returns true if accepting this fragment would place data past an MF=0
+ * (last) fragment. The last fragment fixes the datagram's total length, so
+ * any octet at a higher offset is invalid per the IPv4 (RFC 791) and IPv6
+ * (RFC 8200, Section 4.5) reassembly procedures. */
+static bool
+ipf_is_beyond_last_frag(const struct ipf_frag *frag_list, int last_inuse_idx,
+ uint16_t start_data_byte, uint16_t end_data_byte,
+ bool lf)
+ /* OVS_REQUIRES(ipf_lock) */
+{
+ for (int i = 0; i <= last_inuse_idx; i++) {
+ if (frag_list[i].last_frag
+ && start_data_byte > frag_list[i].end_data_byte) {
+ return true;
+ }
+
+ if (lf && frag_list[i].end_data_byte > end_data_byte) {
+ return true;
+ }
+ }
+
+ return false;
+}
+
/* Drops the entire fragment list: deletes every fragment packet held by the
* list and removes the list from the tracking datastructures. Per RFC 5722
* and RFC 8200, when an overlapping fragment is detected the whole datagram
@@ -926,14 +958,17 @@ ipf_process_frag(struct ipf *ipf, struct ipf_list
*ipf_list,
ipf_list->last_inuse_idx, start_data_byte, end_data_byte);
bool overlap_frag = ipf_is_frag_overlap(ipf_list->frag_list,
ipf_list->last_inuse_idx, start_data_byte, end_data_byte);
+ bool beyond_last = ipf_is_beyond_last_frag(ipf_list->frag_list,
+ ipf_list->last_inuse_idx, start_data_byte, end_data_byte, lf);
int last_inuse_idx = ipf_list->last_inuse_idx;
- if (!duped_frag && !overlap_frag) {
+ if (!duped_frag && !overlap_frag && !beyond_last) {
if (last_inuse_idx < ipf_list->size - 1) {
struct ipf_frag *frag = &ipf_list->frag_list[last_inuse_idx + 1];
frag->pkt = pkt;
frag->start_data_byte = start_data_byte;
frag->end_data_byte = end_data_byte;
+ frag->last_frag = lf;
ipf_list->last_inuse_idx++;
atomic_count_inc(&ipf->nfrag);
ipf_count(ipf, v6, IPF_NFRAGS_ACCEPTED);
@@ -946,11 +981,16 @@ ipf_process_frag(struct ipf *ipf, struct ipf_list
*ipf_list,
* of the fragment list for later reassembly. */
ipf_count(ipf, v6, IPF_NFRAGS_DUPLICATE);
dp_packet_delete(pkt);
- } else {
+ } else if (overlap_frag) {
ipf_count(ipf, v6, IPF_NFRAGS_OVERLAP);
dp_packet_delete(pkt);
ipf_drop_frag_chain(ipf, ipf_list);
+ } else {
+ ipf_count(ipf, v6, IPF_NFRAGS_BEYOND_LAST);
+ dp_packet_delete(pkt);
+ return true;
}
+
return true;
}
@@ -1547,6 +1587,8 @@ ipf_get_status(struct ipf *ipf, struct ipf_status
*ipf_status)
&ipf_status->v4.nfrag_overlap);
atomic_read_relaxed(&ipf->n4frag_cnt[IPF_NFRAGS_DUPLICATE],
&ipf_status->v4.nfrag_duplicate);
+ atomic_read_relaxed(&ipf->n4frag_cnt[IPF_NFRAGS_BEYOND_LAST],
+ &ipf_status->v4.nfrag_beyond_last);
atomic_read_relaxed(&ipf->n4frag_cnt[IPF_NFRAGS_PURGED],
&ipf_status->v4.nfrag_purged);
@@ -1567,6 +1609,8 @@ ipf_get_status(struct ipf *ipf, struct ipf_status
*ipf_status)
&ipf_status->v6.nfrag_overlap);
atomic_read_relaxed(&ipf->n6frag_cnt[IPF_NFRAGS_DUPLICATE],
&ipf_status->v6.nfrag_duplicate);
+ atomic_read_relaxed(&ipf->n6frag_cnt[IPF_NFRAGS_BEYOND_LAST],
+ &ipf_status->v6.nfrag_beyond_last);
atomic_read_relaxed(&ipf->n6frag_cnt[IPF_NFRAGS_PURGED],
&ipf_status->v6.nfrag_purged);
return 0;
diff --git a/lib/ipf.h b/lib/ipf.h
index 775acbf73..81799096e 100644
--- a/lib/ipf.h
+++ b/lib/ipf.h
@@ -30,6 +30,7 @@ struct ipf_proto_status {
uint64_t nfrag_too_large;
uint64_t nfrag_overlap;
uint64_t nfrag_duplicate;
+ uint64_t nfrag_beyond_last;
uint64_t nfrag_purged;
unsigned int min_frag_size;
bool enabled;
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index a427765d5..0fb4170e1 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -5846,6 +5846,80 @@ AT_CHECK([ovs-appctl dpctl/ipf-get-status -m \
OVS_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([ofproto-dpif - fragment handling - reject fragment beyond last])
+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 a non-last fragment
+dnl must 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 last (MF=0) fragment carrying bytes 400..799, then reject a
+dnl new fragment carrying bytes 800..1199, whose data lies entirely beyond the
+dnl end of the last fragment and so must not be reassembled.
+dnl
+dnl Packet 1 (admitted). Last fragment carrying bytes 400..799 (MF clear).
+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 000. .... = Flags: 0x0 (Last fragment)
+dnl ...0 0000 0011 0010 = Fragment Offset: 400
+dnl Time to Live: 64
+dnl Protocol: UDP (17)
+dnl Header Checksum: 0x62f3
+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 00 32 40 11 62 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 beyond last). Fragment carrying bytes 800..1199
+dnl (MF set), starting past packet 1's last-fragment end.
+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 0110 0100 = Fragment Offset: 800
+dnl Time to Live: 64
+dnl Protocol: UDP (17)
+dnl Header Checksum: 0x42c1
+dnl Data (400 bytes)
+ip2="45 00 01 a4 00 20 20 64 40 11 42 c1"
+data2=$(printf '%0*d' 800 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 beyond
last:'], [], [dnl
+ num frag: 1
+ v4 frags accepted: 1
+ v4 frags completed: 0
+ v4 frags beyond last: 1
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
AT_SETUP([ofproto-dpif - handling of malformed TCP packets])
OVS_VSWITCHD_START
add_of_ports br0 1 90
diff --git a/tests/system-userspace-macros.at b/tests/system-userspace-macros.at
index 218cfe8c3..c3f15aa46 100644
--- a/tests/system-userspace-macros.at
+++ b/tests/system-userspace-macros.at
@@ -181,6 +181,7 @@ AT_CHECK([ovs-appctl dpctl/ipf-get-status], [], [dnl
v4 frags too large: 0
v4 frags overlapped: 0
v4 frags duplicate: 0
+ v4 frags beyond last: 0
v4 frags purged: 0
min v6 frag size: 1280
v6 frags accepted: 0
@@ -190,6 +191,7 @@ AT_CHECK([ovs-appctl dpctl/ipf-get-status], [], [dnl
v6 frags too large: 0
v6 frags overlapped: 0
v6 frags duplicate: 0
+ v6 frags beyond last: 0
v6 frags purged: 0
])
])
@@ -215,6 +217,7 @@ AT_CHECK([ovs-appctl dpctl/ipf-get-status --more], [], [dnl
v4 frags too large: 0
v4 frags overlapped: 0
v4 frags duplicate: 0
+ v4 frags beyond last: 0
v4 frags purged: 0
min v6 frag size: 1280
v6 frags accepted: 0
@@ -224,6 +227,7 @@ AT_CHECK([ovs-appctl dpctl/ipf-get-status --more], [], [dnl
v6 frags too large: 0
v6 frags overlapped: 0
v6 frags duplicate: 0
+ v6 frags beyond last: 0
v6 frags purged: 0
Fragment Lists:
@@ -252,6 +256,7 @@ AT_CHECK([ovs-appctl dpctl/ipf-get-status --more], [], [dnl
v4 frags too large: 0
v4 frags overlapped: 0
v4 frags duplicate: 0
+ v4 frags beyond last: 0
v4 frags purged: 0
min v6 frag size: 1280
v6 frags accepted: 30
@@ -261,6 +266,7 @@ AT_CHECK([ovs-appctl dpctl/ipf-get-status --more], [], [dnl
v6 frags too large: 0
v6 frags overlapped: 0
v6 frags duplicate: 0
+ v6 frags beyond last: 0
v6 frags purged: 0
Fragment Lists:
@@ -296,6 +302,7 @@ AT_CHECK([ovs-appctl dpctl/ipf-get-status -m |
FORMAT_FRAG_LIST()], [], [dnl
v4 frags too large: 0
v4 frags overlapped: 0
v4 frags duplicate: 0
+ v4 frags beyond last: 0
v4 frags purged: 0
min v6 frag size: 1280
v6 frags accepted: 0
@@ -305,6 +312,7 @@ AT_CHECK([ovs-appctl dpctl/ipf-get-status -m |
FORMAT_FRAG_LIST()], [], [dnl
v6 frags too large: 0
v6 frags overlapped: 0
v6 frags duplicate: 0
+ v6 frags beyond last: 0
v6 frags purged: 0
Fragment Lists:
--
2.43.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
