1From b6c4a1b204740fe52b32e7f530831a59f4038e20 Mon Sep 17 00:00:00 2001 2From: Alexey Makhalov <amakhalov@vmware.com> 3Date: Thu, 9 Jul 2020 08:10:40 +0000 4Subject: [PATCH] tftp: Do not use priority queue 5MIME-Version: 1.0 6Content-Type: text/plain; charset=UTF-8 7Content-Transfer-Encoding: 8bit 8 9There is not need to reassemble the order of blocks. Per RFC 1350, 10server must wait for the ACK, before sending next block. Data packets 11can be served immediately without putting them to priority queue. 12 13Logic to handle incoming packet is this: 14 - if packet block id equal to expected block id, then 15 process the packet, 16 - if packet block id is less than expected - this is retransmit 17 of old packet, then ACK it and drop the packet, 18 - if packet block id is more than expected - that shouldn't 19 happen, just drop the packet. 20 21It makes the tftp receive path code simpler, smaller and faster. 22As a benefit, this change fixes CID# 73624 and CID# 96690, caused 23by following while loop: 24 25 while (cmp_block (grub_be_to_cpu16 (tftph->u.data.block), data->block + 1) == 0) 26 27where tftph pointer is not moving from one iteration to another, causing 28to serve same packet again. Luckily, double serving didn't happen due to 29data->block++ during the first iteration. 30 31Fixes: CID 73624, CID 96690 32 33Signed-off-by: Alexey Makhalov <amakhalov@vmware.com> 34Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> 35Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com> 36--- 37 grub-core/net/tftp.c | 168 ++++++++++++++----------------------------- 38 1 file changed, 53 insertions(+), 115 deletions(-) 39 40diff --git a/grub-core/net/tftp.c b/grub-core/net/tftp.c 41index 7d90bf66e..b4297bc8d 100644 42--- a/grub-core/net/tftp.c 43+++ b/grub-core/net/tftp.c 44@@ -25,7 +25,6 @@ 45 #include <grub/mm.h> 46 #include <grub/dl.h> 47 #include <grub/file.h> 48-#include <grub/priority_queue.h> 49 #include <grub/i18n.h> 50 51 GRUB_MOD_LICENSE ("GPLv3+"); 52@@ -106,31 +105,8 @@ typedef struct tftp_data 53 int have_oack; 54 struct grub_error_saved save_err; 55 grub_net_udp_socket_t sock; 56- grub_priority_queue_t pq; 57 } *tftp_data_t; 58 59-static int 60-cmp_block (grub_uint16_t a, grub_uint16_t b) 61-{ 62- grub_int16_t i = (grub_int16_t) (a - b); 63- if (i > 0) 64- return +1; 65- if (i < 0) 66- return -1; 67- return 0; 68-} 69- 70-static int 71-cmp (const void *a__, const void *b__) 72-{ 73- struct grub_net_buff *a_ = *(struct grub_net_buff **) a__; 74- struct grub_net_buff *b_ = *(struct grub_net_buff **) b__; 75- struct tftphdr *a = (struct tftphdr *) a_->data; 76- struct tftphdr *b = (struct tftphdr *) b_->data; 77- /* We want the first elements to be on top. */ 78- return -cmp_block (grub_be_to_cpu16 (a->u.data.block), grub_be_to_cpu16 (b->u.data.block)); 79-} 80- 81 static grub_err_t 82 ack (tftp_data_t data, grub_uint64_t block) 83 { 84@@ -207,73 +183,60 @@ tftp_receive (grub_net_udp_socket_t sock __attribute__ ((unused)), 85 return GRUB_ERR_NONE; 86 } 87 88- err = grub_priority_queue_push (data->pq, &nb); 89- if (err) 90- return err; 91- 92- { 93- struct grub_net_buff **nb_top_p, *nb_top; 94- while (1) 95- { 96- nb_top_p = grub_priority_queue_top (data->pq); 97- if (!nb_top_p) 98- return GRUB_ERR_NONE; 99- nb_top = *nb_top_p; 100- tftph = (struct tftphdr *) nb_top->data; 101- if (cmp_block (grub_be_to_cpu16 (tftph->u.data.block), data->block + 1) >= 0) 102- break; 103- ack (data, grub_be_to_cpu16 (tftph->u.data.block)); 104- grub_netbuff_free (nb_top); 105- grub_priority_queue_pop (data->pq); 106- } 107- while (cmp_block (grub_be_to_cpu16 (tftph->u.data.block), data->block + 1) == 0) 108- { 109- unsigned size; 110- 111- grub_priority_queue_pop (data->pq); 112- 113- if (file->device->net->packs.count < 50) 114+ /* Ack old/retransmitted block. */ 115+ if (grub_be_to_cpu16 (tftph->u.data.block) < data->block + 1) 116+ ack (data, grub_be_to_cpu16 (tftph->u.data.block)); 117+ /* Ignore unexpected block. */ 118+ else if (grub_be_to_cpu16 (tftph->u.data.block) > data->block + 1) 119+ grub_dprintf ("tftp", "TFTP unexpected block # %d\n", tftph->u.data.block); 120+ else 121+ { 122+ unsigned size; 123+ 124+ if (file->device->net->packs.count < 50) 125+ { 126 err = ack (data, data->block + 1); 127- else 128- { 129- file->device->net->stall = 1; 130- err = 0; 131- } 132- if (err) 133- return err; 134- 135- err = grub_netbuff_pull (nb_top, sizeof (tftph->opcode) + 136- sizeof (tftph->u.data.block)); 137- if (err) 138- return err; 139- size = nb_top->tail - nb_top->data; 140- 141- data->block++; 142- if (size < data->block_size) 143- { 144- if (data->ack_sent < data->block) 145- ack (data, data->block); 146- file->device->net->eof = 1; 147- file->device->net->stall = 1; 148- grub_net_udp_close (data->sock); 149- data->sock = NULL; 150- } 151- /* Prevent garbage in broken cards. Is it still necessary 152- given that IP implementation has been fixed? 153- */ 154- if (size > data->block_size) 155- { 156- err = grub_netbuff_unput (nb_top, size - data->block_size); 157- if (err) 158- return err; 159- } 160- /* If there is data, puts packet in socket list. */ 161- if ((nb_top->tail - nb_top->data) > 0) 162- grub_net_put_packet (&file->device->net->packs, nb_top); 163- else 164- grub_netbuff_free (nb_top); 165- } 166- } 167+ if (err) 168+ return err; 169+ } 170+ else 171+ file->device->net->stall = 1; 172+ 173+ err = grub_netbuff_pull (nb, sizeof (tftph->opcode) + 174+ sizeof (tftph->u.data.block)); 175+ if (err) 176+ return err; 177+ size = nb->tail - nb->data; 178+ 179+ data->block++; 180+ if (size < data->block_size) 181+ { 182+ if (data->ack_sent < data->block) 183+ ack (data, data->block); 184+ file->device->net->eof = 1; 185+ file->device->net->stall = 1; 186+ grub_net_udp_close (data->sock); 187+ data->sock = NULL; 188+ } 189+ /* 190+ * Prevent garbage in broken cards. Is it still necessary 191+ * given that IP implementation has been fixed? 192+ */ 193+ if (size > data->block_size) 194+ { 195+ err = grub_netbuff_unput (nb, size - data->block_size); 196+ if (err) 197+ return err; 198+ } 199+ /* If there is data, puts packet in socket list. */ 200+ if ((nb->tail - nb->data) > 0) 201+ { 202+ grub_net_put_packet (&file->device->net->packs, nb); 203+ /* Do not free nb. */ 204+ return GRUB_ERR_NONE; 205+ } 206+ } 207+ grub_netbuff_free (nb); 208 return GRUB_ERR_NONE; 209 case TFTP_ERROR: 210 data->have_oack = 1; 211@@ -287,19 +250,6 @@ tftp_receive (grub_net_udp_socket_t sock __attribute__ ((unused)), 212 } 213 } 214 215-static void 216-destroy_pq (tftp_data_t data) 217-{ 218- struct grub_net_buff **nb_p; 219- while ((nb_p = grub_priority_queue_top (data->pq))) 220- { 221- grub_netbuff_free (*nb_p); 222- grub_priority_queue_pop (data->pq); 223- } 224- 225- grub_priority_queue_destroy (data->pq); 226-} 227- 228 static grub_err_t 229 tftp_open (struct grub_file *file, const char *filename) 230 { 231@@ -372,17 +322,9 @@ tftp_open (struct grub_file *file, const char *filename) 232 file->not_easily_seekable = 1; 233 file->data = data; 234 235- data->pq = grub_priority_queue_new (sizeof (struct grub_net_buff *), cmp); 236- if (!data->pq) 237- { 238- grub_free (data); 239- return grub_errno; 240- } 241- 242 err = grub_net_resolve_address (file->device->net->server, &addr); 243 if (err) 244 { 245- destroy_pq (data); 246 grub_free (data); 247 return err; 248 } 249@@ -392,7 +334,6 @@ tftp_open (struct grub_file *file, const char *filename) 250 file); 251 if (!data->sock) 252 { 253- destroy_pq (data); 254 grub_free (data); 255 return grub_errno; 256 } 257@@ -406,7 +347,6 @@ tftp_open (struct grub_file *file, const char *filename) 258 if (err) 259 { 260 grub_net_udp_close (data->sock); 261- destroy_pq (data); 262 grub_free (data); 263 return err; 264 } 265@@ -423,7 +363,6 @@ tftp_open (struct grub_file *file, const char *filename) 266 if (grub_errno) 267 { 268 grub_net_udp_close (data->sock); 269- destroy_pq (data); 270 grub_free (data); 271 return grub_errno; 272 } 273@@ -466,7 +405,6 @@ tftp_close (struct grub_file *file) 274 grub_print_error (); 275 grub_net_udp_close (data->sock); 276 } 277- destroy_pq (data); 278 grub_free (data); 279 return GRUB_ERR_NONE; 280 } 281-- 2822.26.2 283 284