47cf637
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
47cf637
From: Alexey Makhalov <amakhalov@vmware.com>
47cf637
Date: Thu, 9 Jul 2020 08:10:40 +0000
47cf637
Subject: [PATCH] tftp: Do not use priority queue
47cf637
47cf637
There is not need to reassemble the order of blocks. Per RFC 1350,
47cf637
server must wait for the ACK, before sending next block. Data packets
47cf637
can be served immediately without putting them to priority queue.
47cf637
47cf637
Logic to handle incoming packet is this:
47cf637
  - if packet block id equal to expected block id, then
47cf637
    process the packet,
47cf637
  - if packet block id is less than expected - this is retransmit
47cf637
    of old packet, then ACK it and drop the packet,
47cf637
  - if packet block id is more than expected - that shouldn't
47cf637
    happen, just drop the packet.
47cf637
47cf637
It makes the tftp receive path code simpler, smaller and faster.
47cf637
As a benefit, this change fixes CID# 73624 and CID# 96690, caused
47cf637
by following while loop:
47cf637
47cf637
  while (cmp_block (grub_be_to_cpu16 (tftph->u.data.block), data->block + 1) == 0)
47cf637
47cf637
where tftph pointer is not moving from one iteration to another, causing
47cf637
to serve same packet again. Luckily, double serving didn't happen due to
47cf637
data->block++ during the first iteration.
47cf637
47cf637
Fixes: CID 73624, CID 96690
47cf637
47cf637
Signed-off-by: Alexey Makhalov <amakhalov@vmware.com>
47cf637
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
47cf637
Upstream-commit-id: 8316694c4f7
47cf637
---
47cf637
 grub-core/net/tftp.c | 162 ++++++++++++++++-----------------------------------
47cf637
 1 file changed, 50 insertions(+), 112 deletions(-)
47cf637
47cf637
diff --git a/grub-core/net/tftp.c b/grub-core/net/tftp.c
cc2f966
index 4920c3a9783..22badd74316 100644
47cf637
--- a/grub-core/net/tftp.c
47cf637
+++ b/grub-core/net/tftp.c
47cf637
@@ -25,7 +25,6 @@
47cf637
 #include <grub/mm.h>
47cf637
 #include <grub/dl.h>
47cf637
 #include <grub/file.h>
47cf637
-#include <grub/priority_queue.h>
47cf637
 #include <grub/i18n.h>
47cf637
 
47cf637
 GRUB_MOD_LICENSE ("GPLv3+");
47cf637
@@ -106,31 +105,8 @@ typedef struct tftp_data
47cf637
   int have_oack;
47cf637
   struct grub_error_saved save_err;
47cf637
   grub_net_udp_socket_t sock;
47cf637
-  grub_priority_queue_t pq;
47cf637
 } *tftp_data_t;
47cf637
 
47cf637
-static int
47cf637
-cmp_block (grub_uint16_t a, grub_uint16_t b)
47cf637
-{
47cf637
-  grub_int16_t i = (grub_int16_t) (a - b);
47cf637
-  if (i > 0)
47cf637
-    return +1;
47cf637
-  if (i < 0)
47cf637
-    return -1;
47cf637
-  return 0;
47cf637
-}
47cf637
-
47cf637
-static int
47cf637
-cmp (const void *a__, const void *b__)
47cf637
-{
47cf637
-  struct grub_net_buff *a_ = *(struct grub_net_buff **) a__;
47cf637
-  struct grub_net_buff *b_ = *(struct grub_net_buff **) b__;
47cf637
-  struct tftphdr *a = (struct tftphdr *) a_->data;
47cf637
-  struct tftphdr *b = (struct tftphdr *) b_->data;
47cf637
-  /* We want the first elements to be on top.  */
47cf637
-  return -cmp_block (grub_be_to_cpu16 (a->u.data.block), grub_be_to_cpu16 (b->u.data.block));
47cf637
-}
47cf637
-
47cf637
 static grub_err_t
47cf637
 ack (tftp_data_t data, grub_uint64_t block)
47cf637
 {
47cf637
@@ -207,73 +183,60 @@ tftp_receive (grub_net_udp_socket_t sock __attribute__ ((unused)),
47cf637
 	  return GRUB_ERR_NONE;
47cf637
 	}
47cf637
 
47cf637
-      err = grub_priority_queue_push (data->pq, &nb);
47cf637
-      if (err)
47cf637
-	return err;
47cf637
+      /* Ack old/retransmitted block. */
47cf637
+      if (grub_be_to_cpu16 (tftph->u.data.block) < data->block + 1)
47cf637
+	ack (data, grub_be_to_cpu16 (tftph->u.data.block));
47cf637
+      /* Ignore unexpected block. */
47cf637
+      else if (grub_be_to_cpu16 (tftph->u.data.block) > data->block + 1)
47cf637
+	grub_dprintf ("tftp", "TFTP unexpected block # %d\n", tftph->u.data.block);
47cf637
+      else
47cf637
+	{
47cf637
+	  unsigned size;
47cf637
 
47cf637
-      {
47cf637
-	struct grub_net_buff **nb_top_p, *nb_top;
47cf637
-	while (1)
47cf637
-	  {
47cf637
-	    nb_top_p = grub_priority_queue_top (data->pq);
47cf637
-	    if (!nb_top_p)
47cf637
-	      return GRUB_ERR_NONE;
47cf637
-	    nb_top = *nb_top_p;
47cf637
-	    tftph = (struct tftphdr *) nb_top->data;
47cf637
-	    if (cmp_block (grub_be_to_cpu16 (tftph->u.data.block), data->block + 1) >= 0)
47cf637
-	      break;
47cf637
-	    ack (data, grub_be_to_cpu16 (tftph->u.data.block));
47cf637
-	    grub_netbuff_free (nb_top);
47cf637
-	    grub_priority_queue_pop (data->pq);
47cf637
-	  }
47cf637
-	while (cmp_block (grub_be_to_cpu16 (tftph->u.data.block), data->block + 1) == 0)
47cf637
-	  {
47cf637
-	    unsigned size;
47cf637
-
47cf637
-	    grub_priority_queue_pop (data->pq);
47cf637
-
47cf637
-	    if (file->device->net->packs.count < 50)
47cf637
+	  if (file->device->net->packs.count < 50)
47cf637
+	    {
47cf637
 	      err = ack (data, data->block + 1);
47cf637
-	    else
47cf637
-	      {
47cf637
-		file->device->net->stall = 1;
47cf637
-		err = 0;
47cf637
-	      }
47cf637
-	    if (err)
47cf637
-	      return err;
47cf637
+	      if (err)
47cf637
+		return err;
47cf637
+	    }
47cf637
+	  else
47cf637
+	    file->device->net->stall = 1;
47cf637
 
47cf637
-	    err = grub_netbuff_pull (nb_top, sizeof (tftph->opcode) +
47cf637
-				     sizeof (tftph->u.data.block));
47cf637
-	    if (err)
47cf637
-	      return err;
47cf637
-	    size = nb_top->tail - nb_top->data;
47cf637
+	  err = grub_netbuff_pull (nb, sizeof (tftph->opcode) +
47cf637
+				   sizeof (tftph->u.data.block));
47cf637
+	  if (err)
47cf637
+	    return err;
47cf637
+	  size = nb->tail - nb->data;
47cf637
 
47cf637
-	    data->block++;
47cf637
-	    if (size < data->block_size)
47cf637
-	      {
47cf637
-		if (data->ack_sent < data->block)
47cf637
-		  ack (data, data->block);
47cf637
-		file->device->net->eof = 1;
47cf637
-		file->device->net->stall = 1;
47cf637
-		grub_net_udp_close (data->sock);
47cf637
-		data->sock = NULL;
47cf637
-	      }
47cf637
-	    /* Prevent garbage in broken cards. Is it still necessary
47cf637
-	       given that IP implementation has been fixed?
47cf637
-	     */
47cf637
-	    if (size > data->block_size)
47cf637
-	      {
47cf637
-		err = grub_netbuff_unput (nb_top, size - data->block_size);
47cf637
-		if (err)
47cf637
-		  return err;
47cf637
-	      }
47cf637
-	    /* If there is data, puts packet in socket list. */
47cf637
-	    if ((nb_top->tail - nb_top->data) > 0)
47cf637
-	      grub_net_put_packet (&file->device->net->packs, nb_top);
47cf637
-	    else
47cf637
-	      grub_netbuff_free (nb_top);
47cf637
-	  }
47cf637
-      }
47cf637
+	  data->block++;
47cf637
+	  if (size < data->block_size)
47cf637
+	    {
47cf637
+	      if (data->ack_sent < data->block)
47cf637
+		ack (data, data->block);
47cf637
+	      file->device->net->eof = 1;
47cf637
+	      file->device->net->stall = 1;
47cf637
+	      grub_net_udp_close (data->sock);
47cf637
+	      data->sock = NULL;
47cf637
+	    }
47cf637
+	  /*
47cf637
+	   * Prevent garbage in broken cards. Is it still necessary
47cf637
+	   * given that IP implementation has been fixed?
47cf637
+	   */
47cf637
+	  if (size > data->block_size)
47cf637
+	    {
47cf637
+	      err = grub_netbuff_unput (nb, size - data->block_size);
47cf637
+	      if (err)
47cf637
+		return err;
47cf637
+	    }
47cf637
+	  /* If there is data, puts packet in socket list. */
47cf637
+	  if ((nb->tail - nb->data) > 0)
47cf637
+	    {
47cf637
+	      grub_net_put_packet (&file->device->net->packs, nb);
47cf637
+	      /* Do not free nb. */
47cf637
+	      return GRUB_ERR_NONE;
47cf637
+	    }
47cf637
+	}
47cf637
+      grub_netbuff_free (nb);
47cf637
       return GRUB_ERR_NONE;
47cf637
     case TFTP_ERROR:
47cf637
       data->have_oack = 1;
47cf637
@@ -287,19 +250,6 @@ tftp_receive (grub_net_udp_socket_t sock __attribute__ ((unused)),
47cf637
     }
47cf637
 }
47cf637
 
47cf637
-static void
47cf637
-destroy_pq (tftp_data_t data)
47cf637
-{
47cf637
-  struct grub_net_buff **nb_p;
47cf637
-  while ((nb_p = grub_priority_queue_top (data->pq)))
47cf637
-    {
47cf637
-      grub_netbuff_free (*nb_p);
47cf637
-      grub_priority_queue_pop (data->pq);
47cf637
-    }
47cf637
-
47cf637
-  grub_priority_queue_destroy (data->pq);
47cf637
-}
47cf637
-
47cf637
 /* Create a normalized copy of the filename.
47cf637
    Compress any string of consecutive forward slashes to a single forward
47cf637
    slash. */
47cf637
@@ -395,13 +345,6 @@ tftp_open (struct grub_file *file, const char *filename)
47cf637
   file->not_easily_seekable = 1;
47cf637
   file->data = data;
47cf637
 
47cf637
-  data->pq = grub_priority_queue_new (sizeof (struct grub_net_buff *), cmp);
47cf637
-  if (!data->pq)
47cf637
-    {
47cf637
-      grub_free (data);
47cf637
-      return grub_errno;
47cf637
-    }
47cf637
-
cc2f966
   grub_dprintf ("tftp", "resolving address for %s\n", file->device->net->server);
47cf637
   err = grub_net_resolve_address (file->device->net->server, &addr);
47cf637
   if (err)
47cf637
@@ -410,7 +353,6 @@ tftp_open (struct grub_file *file, const char *filename)
47cf637
       grub_dprintf ("tftp", "file_size is %llu, block_size is %llu\n",
47cf637
 		    (unsigned long long)data->file_size,
47cf637
 		    (unsigned long long)data->block_size);
47cf637
-      destroy_pq (data);
47cf637
       grub_free (data);
47cf637
       return err;
47cf637
     }
47cf637
@@ -422,7 +364,6 @@ tftp_open (struct grub_file *file, const char *filename)
47cf637
   if (!data->sock)
47cf637
     {
cc2f966
       grub_dprintf ("tftp", "connection failed\n");
47cf637
-      destroy_pq (data);
47cf637
       grub_free (data);
47cf637
       return grub_errno;
47cf637
     }
47cf637
@@ -436,7 +377,6 @@ tftp_open (struct grub_file *file, const char *filename)
47cf637
       if (err)
47cf637
 	{
47cf637
 	  grub_net_udp_close (data->sock);
47cf637
-	  destroy_pq (data);
47cf637
 	  grub_free (data);
47cf637
 	  return err;
47cf637
 	}
47cf637
@@ -453,7 +393,6 @@ tftp_open (struct grub_file *file, const char *filename)
47cf637
   if (grub_errno)
47cf637
     {
47cf637
       grub_net_udp_close (data->sock);
47cf637
-      destroy_pq (data);
47cf637
       grub_free (data);
47cf637
       return grub_errno;
47cf637
     }
47cf637
@@ -496,7 +435,6 @@ tftp_close (struct grub_file *file)
47cf637
 	grub_print_error ();
47cf637
       grub_net_udp_close (data->sock);
47cf637
     }
47cf637
-  destroy_pq (data);
47cf637
   grub_free (data);
47cf637
   return GRUB_ERR_NONE;
47cf637
 }