dcavalca / rpms / grub2

Forked from rpms/grub2 3 years ago
Clone

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

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