When conntrack is reassembling packet fragments, the same reassembly
context can be shared across multiple threads handling different packets
simultaneously. Once a full packet is assembled, it is added to a packet
batch for processing, this is most likely the batch that added it in the
first place, but that isn't a guarantee.
The packets in these batches should be segregated by network protocol
versuib (ipv4 vs ipv6) for conntrack defragmentation to function
appropriately. However, there are conditions where we would add a
reassembled packet of one type to a batch of another.
This change introduces checks to make sure that reassembled or expired
fragments are only added to packet batches of the same type. It also
makes a best effort attempt to make sure the defragmented packet is
inserted into the current batch.
Fixes: 4ea96698f667 ("Userspace datapath: Add fragmentation handling.")
Reported-at: https://issues.redhat.com/browse/FDP-560
Signed-off-by: Mike Pattrick <[email protected]>
---
Note: This solution is far from perfect, ipf.c can still insert packets
into more or less arbitrary batches but this bug fix is needed to avoid a
memory overrun and should insert packets into the proper batch in the
common case. I'm working on a more correct solution but it changes how
fragments are fundimentally handled, and couldn't be considered a bug fix.
---
lib/ipf.c | 40 +++++++++++++++++++++++++++++++---------
1 file changed, 31 insertions(+), 9 deletions(-)
diff --git a/lib/ipf.c b/lib/ipf.c
index 7d74e2c13..90c819d63 100644
--- a/lib/ipf.c
+++ b/lib/ipf.c
@@ -506,13 +506,15 @@ ipf_reassemble_v6_frags(struct ipf_list *ipf_list)
}
/* Called when a frag list state transitions to another state. This is
- * triggered by new fragment for the list being received.*/
-static void
+* triggered by new fragment for the list being received. Returns a reassembled
+* packet if this fragment has completed one. */
+static struct reassembled_pkt *
ipf_list_state_transition(struct ipf *ipf, struct ipf_list *ipf_list,
bool ff, bool lf, bool v6)
OVS_REQUIRES(ipf->ipf_lock)
{
enum ipf_list_state curr_state = ipf_list->state;
+ struct reassembled_pkt *ret = NULL;
enum ipf_list_state next_state;
switch (curr_state) {
case IPF_LIST_STATE_UNUSED:
@@ -562,12 +564,15 @@ ipf_list_state_transition(struct ipf *ipf, struct
ipf_list *ipf_list,
ipf_reassembled_list_add(&ipf->reassembled_pkt_list, rp);
ipf_expiry_list_remove(ipf_list);
next_state = IPF_LIST_STATE_COMPLETED;
+ ret = rp;
} else {
next_state = IPF_LIST_STATE_REASS_FAIL;
}
}
}
ipf_list->state = next_state;
+
+ return ret;
}
/* Some sanity checks are redundant, but prudent, in case code paths for
@@ -799,7 +804,8 @@ ipf_is_frag_duped(const struct ipf_frag *frag_list, int
last_inuse_idx,
static bool
ipf_process_frag(struct ipf *ipf, struct ipf_list *ipf_list,
struct dp_packet *pkt, uint16_t start_data_byte,
- uint16_t end_data_byte, bool ff, bool lf, bool v6)
+ uint16_t end_data_byte, bool ff, bool lf, bool v6,
+ struct reassembled_pkt **rp)
OVS_REQUIRES(ipf->ipf_lock)
{
bool duped_frag = ipf_is_frag_duped(ipf_list->frag_list,
@@ -820,13 +826,14 @@ ipf_process_frag(struct ipf *ipf, struct ipf_list
*ipf_list,
ipf_list->last_inuse_idx++;
atomic_count_inc(&ipf->nfrag);
ipf_count(ipf, v6, IPF_NFRAGS_ACCEPTED);
- ipf_list_state_transition(ipf, ipf_list, ff, lf, v6);
+ *rp = ipf_list_state_transition(ipf, ipf_list, ff, lf, v6);
} else {
OVS_NOT_REACHED();
}
} else {
ipf_count(ipf, v6, IPF_NFRAGS_OVERLAP);
pkt->md.ct_state = CS_INVALID;
+ *rp = NULL;
return false;
}
return true;
@@ -853,7 +860,8 @@ ipf_list_init(struct ipf_list *ipf_list, struct
ipf_list_key *key,
* to a list of fragemnts. */
static bool
ipf_handle_frag(struct ipf *ipf, struct dp_packet *pkt, ovs_be16 dl_type,
- uint16_t zone, long long now, uint32_t hash_basis)
+ uint16_t zone, long long now, uint32_t hash_basis,
+ struct reassembled_pkt **rp)
OVS_REQUIRES(ipf->ipf_lock)
{
struct ipf_list_key key;
@@ -922,7 +930,7 @@ ipf_handle_frag(struct ipf *ipf, struct dp_packet *pkt,
ovs_be16 dl_type,
}
return ipf_process_frag(ipf, ipf_list, pkt, start_data_byte,
- end_data_byte, ff, lf, v6);
+ end_data_byte, ff, lf, v6, rp);
}
/* Filters out fragments from a batch of fragments and adjust the batch. */
@@ -933,6 +941,7 @@ ipf_extract_frags_from_batch(struct ipf *ipf, struct
dp_packet_batch *pb,
{
const size_t pb_cnt = dp_packet_batch_size(pb);
int pb_idx; /* Index in a packet batch. */
+ struct reassembled_pkt *rp;
struct dp_packet *pkt;
DP_PACKET_BATCH_REFILL_FOR_EACH (pb_idx, pb_cnt, pkt, pb) {
@@ -943,9 +952,14 @@ ipf_extract_frags_from_batch(struct ipf *ipf, struct
dp_packet_batch *pb,
ipf_is_valid_v6_frag(ipf, pkt)))) {
ovs_mutex_lock(&ipf->ipf_lock);
- if (!ipf_handle_frag(ipf, pkt, dl_type, zone, now, hash_basis)) {
+ if (!ipf_handle_frag(ipf, pkt, dl_type, zone, now, hash_basis,
+ &rp)) {
dp_packet_batch_refill(pb, pkt, pb_idx);
} else {
+ if (rp && !dp_packet_batch_is_full(pb)) {
+ dp_packet_batch_refill(pb, rp->pkt, pb_idx);
+ rp->list->reass_execute_ctx = rp->pkt;
+ }
dp_packet_delete(pkt);
}
ovs_mutex_unlock(&ipf->ipf_lock);
@@ -1063,6 +1077,9 @@ ipf_send_completed_frags(struct ipf *ipf, struct
dp_packet_batch *pb,
struct ipf_list *ipf_list;
LIST_FOR_EACH_SAFE (ipf_list, list_node, &ipf->frag_complete_list) {
+ if ((ipf_list->key.dl_type == htons(ETH_TYPE_IPV6)) != v6) {
+ continue;
+ }
if (ipf_send_frags_in_list(ipf, ipf_list, pb, IPF_FRAG_COMPLETED_LIST,
v6, now)) {
ipf_completed_list_clean(&ipf->frag_lists, ipf_list);
@@ -1096,6 +1113,9 @@ ipf_send_expired_frags(struct ipf *ipf, struct
dp_packet_batch *pb,
size_t lists_removed = 0;
LIST_FOR_EACH_SAFE (ipf_list, list_node, &ipf->frag_exp_list) {
+ if ((ipf_list->key.dl_type == htons(ETH_TYPE_IPV6)) != v6) {
+ continue;
+ }
if (now <= ipf_list->expiration ||
lists_removed >= IPF_FRAG_LIST_MAX_EXPIRED) {
break;
@@ -1116,7 +1136,8 @@ ipf_send_expired_frags(struct ipf *ipf, struct
dp_packet_batch *pb,
/* Adds a reassmebled packet to a packet batch to be processed by the caller.
*/
static void
-ipf_execute_reass_pkts(struct ipf *ipf, struct dp_packet_batch *pb)
+ipf_execute_reass_pkts(struct ipf *ipf, struct dp_packet_batch *pb,
+ ovs_be16 dl_type)
{
if (ovs_list_is_empty(&ipf->reassembled_pkt_list)) {
return;
@@ -1127,6 +1148,7 @@ ipf_execute_reass_pkts(struct ipf *ipf, struct
dp_packet_batch *pb)
LIST_FOR_EACH_SAFE (rp, rp_list_node, &ipf->reassembled_pkt_list) {
if (!rp->list->reass_execute_ctx &&
+ rp->list->key.dl_type == dl_type &&
ipf_dp_packet_batch_add(pb, rp->pkt, false)) {
rp->list->reass_execute_ctx = rp->pkt;
}
@@ -1237,7 +1259,7 @@ ipf_preprocess_conntrack(struct ipf *ipf, struct
dp_packet_batch *pb,
}
if (ipf_get_enabled(ipf) || atomic_count_get(&ipf->nfrag)) {
- ipf_execute_reass_pkts(ipf, pb);
+ ipf_execute_reass_pkts(ipf, pb, dl_type);
}
}
--
2.39.3
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
