From bbb9d16a452bbad7b104a7800d18fb61e97e399f Mon Sep 17 00:00:00 2001 From: "Fabio M. Di Nitto" Date: Sat, 28 Nov 2015 09:13:33 +0100 Subject: [PATCH] [PMTUd] Fix crash and calculation when using crypto most of the problem was around incorrect typing of variables and incorrect calculation when using signing without encryption. Signed-off-by: Fabio M. Di Nitto --- TODO | 1 - libknet/internals.h | 8 ++++---- libknet/threads_pmtud.c | 33 ++++++++++++++++++++------------- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/TODO b/TODO index 3b73fb4..d60c98c 100644 --- a/TODO +++ b/TODO @@ -8,7 +8,6 @@ link/host level: - (issue) implement packet fragmentation?, is the only solution to avoid conflicts with tun/tap mtu setting and various others buffer sizes. - - (issue) Fix PMTUd with encryption / signing. - (rfe) compress: should only compress user data, we will add a bit in the data header to indicate if the pckt is compressed or not (save time). this approach allow runtime change of compress. diff --git a/libknet/internals.h b/libknet/internals.h index 23210b7..31605f6 100644 --- a/libknet/internals.h +++ b/libknet/internals.h @@ -53,10 +53,10 @@ struct knet_link { uint8_t received_pong; struct timespec ping_last; /* used by PMTUD thread as temp per-link variables and should always contain the onwire_len value! */ - uint16_t last_good_mtu; - uint16_t last_bad_mtu; - uint16_t last_sent_mtu; - uint16_t last_recv_mtu; + uint32_t last_good_mtu; + uint32_t last_bad_mtu; + uint32_t last_sent_mtu; + uint32_t last_recv_mtu; }; #define KNET_CBUFFER_SIZE 4096 diff --git a/libknet/threads_pmtud.c b/libknet/threads_pmtud.c index abf5d17..1d5d9f8 100644 --- a/libknet/threads_pmtud.c +++ b/libknet/threads_pmtud.c @@ -38,6 +38,7 @@ static void _handle_check_pmtud(knet_handle_t knet_h, struct knet_host *dst_host mutex_retry_limit = 0; failsafe = 0; + pad_len = 0; dst_link->last_bad_mtu = 0; @@ -87,22 +88,25 @@ restart: if (knet_h->crypto_instance) { - pad_len = knet_h->sec_block_size - (data_len % knet_h->sec_block_size); - if (pad_len == knet_h->sec_block_size) { - pad_len = 0; + if (knet_h->sec_block_size) { + pad_len = knet_h->sec_block_size - (data_len % knet_h->sec_block_size); + if (pad_len == knet_h->sec_block_size) { + pad_len = 0; + } + data_len = data_len + pad_len; } - data_len = data_len + pad_len; - data_len = data_len + (knet_h->sec_hash_size + knet_h->sec_salt_size + knet_h->sec_block_size); - while (data_len + overhead_len >= max_mtu_len) { - data_len = data_len - knet_h->sec_block_size; + if (knet_h->sec_block_size) { + while (data_len + overhead_len >= max_mtu_len) { + data_len = data_len - knet_h->sec_block_size; + } } if (dst_link->last_bad_mtu) { while (data_len + overhead_len >= dst_link->last_bad_mtu) { - data_len = data_len - knet_h->sec_block_size; + data_len = data_len - (knet_h->sec_hash_size + knet_h->sec_salt_size + knet_h->sec_block_size); } } @@ -192,7 +196,7 @@ restart: } else { int found_mtu = 0; - if (knet_h->crypto_instance) { + if (knet_h->sec_block_size) { if ((onwire_len + knet_h->sec_block_size >= max_mtu_len) || ((dst_link->last_bad_mtu) && (dst_link->last_bad_mtu <= (onwire_len + knet_h->sec_block_size)))) { found_mtu = 1; @@ -234,6 +238,8 @@ void *_handle_pmtud_link_thread(void *data) int ret, have_timer; unsigned int old_interval; + knet_h->link_mtu = KNET_PMTUD_MIN_MTU_V4; + /* preparing pmtu buffer */ knet_h->pmtudbuf->kh_version = KNET_HEADER_VERSION; knet_h->pmtudbuf->kh_type = KNET_HEADER_TYPE_PMTUD; @@ -281,7 +287,7 @@ timer_restart: sleep(knet_h->pmtud_interval); } - min_mtu = KNET_PMTUD_SIZE_V6; + min_mtu = KNET_PMTUD_SIZE_V4; have_mtu = 0; if (knet_h->pmtud_fini_requested) @@ -308,10 +314,10 @@ timer_restart: log_debug(knet_h, KNET_SUB_PMTUD_T, "Starting PMTUD for host: %u link: %u", dst_host->host_id, link_idx); _handle_check_pmtud(knet_h, dst_host, dst_link); if ((saved_pmtud) && (saved_pmtud != dst_link->status.mtu)) { - log_info(knet_h, KNET_SUB_PMTUD_T, "PMTUD change for host: %u link: %u from %u to %u", + log_info(knet_h, KNET_SUB_PMTUD_T, "PMTUD link change for host: %u link: %u from %u to %u", dst_host->host_id, link_idx, saved_pmtud, dst_link->status.mtu); } - log_debug(knet_h, KNET_SUB_PMTUD_T, "PMTUD completed for host: %u link: %u current mtu: %u", + log_debug(knet_h, KNET_SUB_PMTUD_T, "PMTUD completed for host: %u link: %u current link mtu: %u", dst_host->host_id, link_idx, dst_link->status.mtu); if (dst_link->status.mtu < min_mtu) { min_mtu = dst_link->status.mtu; @@ -322,9 +328,10 @@ timer_restart: if (have_mtu) { if (knet_h->link_mtu != min_mtu) { - log_info(knet_h, KNET_SUB_PMTUD_T, "Global MTU changed from: %u to %u", knet_h->link_mtu, min_mtu); + log_info(knet_h, KNET_SUB_PMTUD_T, "Global link MTU changed from: %u to %u", knet_h->link_mtu, min_mtu); knet_h->link_mtu = min_mtu; knet_h->data_mtu = min_mtu - KNET_HEADER_ALL_SIZE - knet_h->sec_header_size; + log_info(knet_h, KNET_SUB_PMTUD_T, "Global data MTU changed to: %u", knet_h->data_mtu); if (knet_h->pmtud_notify_fn) { knet_h->pmtud_notify_fn(knet_h->pmtud_notify_fn_private_data, -- 2.39.5