123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267268269270271272273274275276277278279280281282283 |
- From b6c4a1b204740fe52b32e7f530831a59f4038e20 Mon Sep 17 00:00:00 2001
- From: Alexey Makhalov <amakhalov@vmware.com>
- Date: Thu, 9 Jul 2020 08:10:40 +0000
- Subject: [PATCH] tftp: Do not use priority queue
- MIME-Version: 1.0
- Content-Type: text/plain; charset=UTF-8
- Content-Transfer-Encoding: 8bit
- There is not need to reassemble the order of blocks. Per RFC 1350,
- server must wait for the ACK, before sending next block. Data packets
- can be served immediately without putting them to priority queue.
- Logic to handle incoming packet is this:
- - if packet block id equal to expected block id, then
- process the packet,
- - if packet block id is less than expected - this is retransmit
- of old packet, then ACK it and drop the packet,
- - if packet block id is more than expected - that shouldn't
- happen, just drop the packet.
- It makes the tftp receive path code simpler, smaller and faster.
- As a benefit, this change fixes CID# 73624 and CID# 96690, caused
- by following while loop:
- while (cmp_block (grub_be_to_cpu16 (tftph->u.data.block), data->block + 1) == 0)
- where tftph pointer is not moving from one iteration to another, causing
- to serve same packet again. Luckily, double serving didn't happen due to
- data->block++ during the first iteration.
- Fixes: CID 73624, CID 96690
- Signed-off-by: Alexey Makhalov <amakhalov@vmware.com>
- Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
- Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com>
- ---
- grub-core/net/tftp.c | 168 ++++++++++++++-----------------------------
- 1 file changed, 53 insertions(+), 115 deletions(-)
- diff --git a/grub-core/net/tftp.c b/grub-core/net/tftp.c
- index 7d90bf66e..b4297bc8d 100644
- --- a/grub-core/net/tftp.c
- +++ b/grub-core/net/tftp.c
- @@ -25,7 +25,6 @@
- #include <grub/mm.h>
- #include <grub/dl.h>
- #include <grub/file.h>
- -#include <grub/priority_queue.h>
- #include <grub/i18n.h>
-
- GRUB_MOD_LICENSE ("GPLv3+");
- @@ -106,31 +105,8 @@ typedef struct tftp_data
- int have_oack;
- struct grub_error_saved save_err;
- grub_net_udp_socket_t sock;
- - grub_priority_queue_t pq;
- } *tftp_data_t;
-
- -static int
- -cmp_block (grub_uint16_t a, grub_uint16_t b)
- -{
- - grub_int16_t i = (grub_int16_t) (a - b);
- - if (i > 0)
- - return +1;
- - if (i < 0)
- - return -1;
- - return 0;
- -}
- -
- -static int
- -cmp (const void *a__, const void *b__)
- -{
- - struct grub_net_buff *a_ = *(struct grub_net_buff **) a__;
- - struct grub_net_buff *b_ = *(struct grub_net_buff **) b__;
- - struct tftphdr *a = (struct tftphdr *) a_->data;
- - struct tftphdr *b = (struct tftphdr *) b_->data;
- - /* We want the first elements to be on top. */
- - return -cmp_block (grub_be_to_cpu16 (a->u.data.block), grub_be_to_cpu16 (b->u.data.block));
- -}
- -
- static grub_err_t
- ack (tftp_data_t data, grub_uint64_t block)
- {
- @@ -207,73 +183,60 @@ tftp_receive (grub_net_udp_socket_t sock __attribute__ ((unused)),
- return GRUB_ERR_NONE;
- }
-
- - err = grub_priority_queue_push (data->pq, &nb);
- - if (err)
- - return err;
- -
- - {
- - struct grub_net_buff **nb_top_p, *nb_top;
- - while (1)
- - {
- - nb_top_p = grub_priority_queue_top (data->pq);
- - if (!nb_top_p)
- - return GRUB_ERR_NONE;
- - nb_top = *nb_top_p;
- - tftph = (struct tftphdr *) nb_top->data;
- - if (cmp_block (grub_be_to_cpu16 (tftph->u.data.block), data->block + 1) >= 0)
- - break;
- - ack (data, grub_be_to_cpu16 (tftph->u.data.block));
- - grub_netbuff_free (nb_top);
- - grub_priority_queue_pop (data->pq);
- - }
- - while (cmp_block (grub_be_to_cpu16 (tftph->u.data.block), data->block + 1) == 0)
- - {
- - unsigned size;
- -
- - grub_priority_queue_pop (data->pq);
- -
- - if (file->device->net->packs.count < 50)
- + /* Ack old/retransmitted block. */
- + if (grub_be_to_cpu16 (tftph->u.data.block) < data->block + 1)
- + ack (data, grub_be_to_cpu16 (tftph->u.data.block));
- + /* Ignore unexpected block. */
- + else if (grub_be_to_cpu16 (tftph->u.data.block) > data->block + 1)
- + grub_dprintf ("tftp", "TFTP unexpected block # %d\n", tftph->u.data.block);
- + else
- + {
- + unsigned size;
- +
- + if (file->device->net->packs.count < 50)
- + {
- err = ack (data, data->block + 1);
- - else
- - {
- - file->device->net->stall = 1;
- - err = 0;
- - }
- - if (err)
- - return err;
- -
- - err = grub_netbuff_pull (nb_top, sizeof (tftph->opcode) +
- - sizeof (tftph->u.data.block));
- - if (err)
- - return err;
- - size = nb_top->tail - nb_top->data;
- -
- - data->block++;
- - if (size < data->block_size)
- - {
- - if (data->ack_sent < data->block)
- - ack (data, data->block);
- - file->device->net->eof = 1;
- - file->device->net->stall = 1;
- - grub_net_udp_close (data->sock);
- - data->sock = NULL;
- - }
- - /* Prevent garbage in broken cards. Is it still necessary
- - given that IP implementation has been fixed?
- - */
- - if (size > data->block_size)
- - {
- - err = grub_netbuff_unput (nb_top, size - data->block_size);
- - if (err)
- - return err;
- - }
- - /* If there is data, puts packet in socket list. */
- - if ((nb_top->tail - nb_top->data) > 0)
- - grub_net_put_packet (&file->device->net->packs, nb_top);
- - else
- - grub_netbuff_free (nb_top);
- - }
- - }
- + if (err)
- + return err;
- + }
- + else
- + file->device->net->stall = 1;
- +
- + err = grub_netbuff_pull (nb, sizeof (tftph->opcode) +
- + sizeof (tftph->u.data.block));
- + if (err)
- + return err;
- + size = nb->tail - nb->data;
- +
- + data->block++;
- + if (size < data->block_size)
- + {
- + if (data->ack_sent < data->block)
- + ack (data, data->block);
- + file->device->net->eof = 1;
- + file->device->net->stall = 1;
- + grub_net_udp_close (data->sock);
- + data->sock = NULL;
- + }
- + /*
- + * Prevent garbage in broken cards. Is it still necessary
- + * given that IP implementation has been fixed?
- + */
- + if (size > data->block_size)
- + {
- + err = grub_netbuff_unput (nb, size - data->block_size);
- + if (err)
- + return err;
- + }
- + /* If there is data, puts packet in socket list. */
- + if ((nb->tail - nb->data) > 0)
- + {
- + grub_net_put_packet (&file->device->net->packs, nb);
- + /* Do not free nb. */
- + return GRUB_ERR_NONE;
- + }
- + }
- + grub_netbuff_free (nb);
- return GRUB_ERR_NONE;
- case TFTP_ERROR:
- data->have_oack = 1;
- @@ -287,19 +250,6 @@ tftp_receive (grub_net_udp_socket_t sock __attribute__ ((unused)),
- }
- }
-
- -static void
- -destroy_pq (tftp_data_t data)
- -{
- - struct grub_net_buff **nb_p;
- - while ((nb_p = grub_priority_queue_top (data->pq)))
- - {
- - grub_netbuff_free (*nb_p);
- - grub_priority_queue_pop (data->pq);
- - }
- -
- - grub_priority_queue_destroy (data->pq);
- -}
- -
- static grub_err_t
- tftp_open (struct grub_file *file, const char *filename)
- {
- @@ -372,17 +322,9 @@ tftp_open (struct grub_file *file, const char *filename)
- file->not_easily_seekable = 1;
- file->data = data;
-
- - data->pq = grub_priority_queue_new (sizeof (struct grub_net_buff *), cmp);
- - if (!data->pq)
- - {
- - grub_free (data);
- - return grub_errno;
- - }
- -
- err = grub_net_resolve_address (file->device->net->server, &addr);
- if (err)
- {
- - destroy_pq (data);
- grub_free (data);
- return err;
- }
- @@ -392,7 +334,6 @@ tftp_open (struct grub_file *file, const char *filename)
- file);
- if (!data->sock)
- {
- - destroy_pq (data);
- grub_free (data);
- return grub_errno;
- }
- @@ -406,7 +347,6 @@ tftp_open (struct grub_file *file, const char *filename)
- if (err)
- {
- grub_net_udp_close (data->sock);
- - destroy_pq (data);
- grub_free (data);
- return err;
- }
- @@ -423,7 +363,6 @@ tftp_open (struct grub_file *file, const char *filename)
- if (grub_errno)
- {
- grub_net_udp_close (data->sock);
- - destroy_pq (data);
- grub_free (data);
- return grub_errno;
- }
- @@ -466,7 +405,6 @@ tftp_close (struct grub_file *file)
- grub_print_error ();
- grub_net_udp_close (data->sock);
- }
- - destroy_pq (data);
- grub_free (data);
- return GRUB_ERR_NONE;
- }
- --
- 2.26.2
|