vbatts/grub
1
0
Fork 0
Code Issues Pull requests Releases Wiki Activity

tftp: Do not use priority queue

Browse source 
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>
This commit is contained in:
Alexey Makhalov 2020-07-09 08:10:40 +00:00 committed by Daniel Kiper
parent f8ad7a3dd8
commit 781b3e5efc
1 changed files with 50 additions and 112 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

162
grub-core/net/tftp.c
View file

@ -25,7 +25,6 @@
#include <grub/mm.h> #include <grub/mm.h>
#include <grub/dl.h> #include <grub/dl.h>
#include <grub/file.h> #include <grub/file.h>
#include <grub/priority_queue.h>
#include <grub/i18n.h> #include <grub/i18n.h>
GRUB_MOD_LICENSE ("GPLv3+"); GRUB_MOD_LICENSE ("GPLv3+");
@ -106,31 +105,8 @@ typedef struct tftp_data
int have_oack; int have_oack;
struct grub_error_saved save_err; struct grub_error_saved save_err;
grub_net_udp_socket_t sock; grub_net_udp_socket_t sock;
grub_priority_queue_t pq;
} *tftp_data_t; } *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 static grub_err_t
ack (tftp_data_t data, grub_uint64_t block) 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; return GRUB_ERR_NONE;
} }
err = grub_priority_queue_push (data->pq, &nb); /* Ack old/retransmitted block. */
if (err) if (grub_be_to_cpu16 (tftph->u.data.block) < data->block + 1)
return err; 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)
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)
err = ack (data, data->block + 1); err = ack (data, data->block + 1);
else if (err)
{ return err;
file->device->net->stall = 1; }
err = 0; else
} file->device->net->stall = 1;
if (err)
return err;
err = grub_netbuff_pull (nb_top, sizeof (tftph->opcode) + err = grub_netbuff_pull (nb, sizeof (tftph->opcode) +
sizeof (tftph->u.data.block)); sizeof (tftph->u.data.block));
if (err) if (err)
return err; return err;
size = nb_top->tail - nb_top->data; size = nb->tail - nb->data;
data->block++; data->block++;
if (size < data->block_size) if (size < data->block_size)
{ {
if (data->ack_sent < data->block) if (data->ack_sent < data->block)
ack (data, data->block); ack (data, data->block);
file->device->net->eof = 1; file->device->net->eof = 1;
file->device->net->stall = 1; file->device->net->stall = 1;
grub_net_udp_close (data->sock); grub_net_udp_close (data->sock);
data->sock = NULL; data->sock = NULL;
} }
/* Prevent garbage in broken cards. Is it still necessary /*
given that IP implementation has been fixed? * Prevent garbage in broken cards. Is it still necessary
*/ * given that IP implementation has been fixed?
if (size > data->block_size) */
{ if (size > data->block_size)
err = grub_netbuff_unput (nb_top, size - data->block_size); {
if (err) err = grub_netbuff_unput (nb, size - data->block_size);
return err; if (err)
} return err;
/* If there is data, puts packet in socket list. */ }
if ((nb_top->tail - nb_top->data) > 0) /* If there is data, puts packet in socket list. */
grub_net_put_packet (&file->device->net->packs, nb_top); if ((nb->tail - nb->data) > 0)
else {
grub_netbuff_free (nb_top); 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; return GRUB_ERR_NONE;
case TFTP_ERROR: case TFTP_ERROR:
data->have_oack = 1; 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);
}
/* /*
* Create a normalized copy of the filename. Compress any string of consecutive * Create a normalized copy of the filename. Compress any string of consecutive
* forward slashes to a single forward slash. * forward slashes to a single forward slash.
@ -397,17 +347,9 @@ tftp_open (struct grub_file *file, const char *filename)
file->not_easily_seekable = 1; file->not_easily_seekable = 1;
file->data = data; 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); err = grub_net_resolve_address (file->device->net->server, &addr);
if (err) if (err)
{ {
destroy_pq (data);
grub_free (data); grub_free (data);
return err; return err;
} }
@ -417,7 +359,6 @@ tftp_open (struct grub_file *file, const char *filename)
file); file);
if (!data->sock) if (!data->sock)
{ {
destroy_pq (data);
grub_free (data); grub_free (data);
return grub_errno; return grub_errno;
} }
@ -431,7 +372,6 @@ tftp_open (struct grub_file *file, const char *filename)
if (err) if (err)
{ {
grub_net_udp_close (data->sock); grub_net_udp_close (data->sock);
destroy_pq (data);
grub_free (data); grub_free (data);
return err; return err;
} }
@ -448,7 +388,6 @@ tftp_open (struct grub_file *file, const char *filename)
if (grub_errno) if (grub_errno)
{ {
grub_net_udp_close (data->sock); grub_net_udp_close (data->sock);
destroy_pq (data);
grub_free (data); grub_free (data);
return grub_errno; return grub_errno;
} }
@ -491,7 +430,6 @@ tftp_close (struct grub_file *file)
grub_print_error (); grub_print_error ();
grub_net_udp_close (data->sock); grub_net_udp_close (data->sock);
} }
destroy_pq (data);
grub_free (data); grub_free (data);
return GRUB_ERR_NONE; return GRUB_ERR_NONE;
} }