Blame SOURCES/0294-tftp-Do-not-use-priority-queue.patch

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