0015-tftp-Do-not-use-priority-queue.patch 7.9 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267268269270271272273274275276277278279280281282283
  1. From b6c4a1b204740fe52b32e7f530831a59f4038e20 Mon Sep 17 00:00:00 2001
  2. From: Alexey Makhalov <amakhalov@vmware.com>
  3. Date: Thu, 9 Jul 2020 08:10:40 +0000
  4. Subject: [PATCH] tftp: Do not use priority queue
  5. MIME-Version: 1.0
  6. Content-Type: text/plain; charset=UTF-8
  7. Content-Transfer-Encoding: 8bit
  8. There is not need to reassemble the order of blocks. Per RFC 1350,
  9. server must wait for the ACK, before sending next block. Data packets
  10. can be served immediately without putting them to priority queue.
  11. Logic to handle incoming packet is this:
  12. - if packet block id equal to expected block id, then
  13. process the packet,
  14. - if packet block id is less than expected - this is retransmit
  15. of old packet, then ACK it and drop the packet,
  16. - if packet block id is more than expected - that shouldn't
  17. happen, just drop the packet.
  18. It makes the tftp receive path code simpler, smaller and faster.
  19. As a benefit, this change fixes CID# 73624 and CID# 96690, caused
  20. by following while loop:
  21. while (cmp_block (grub_be_to_cpu16 (tftph->u.data.block), data->block + 1) == 0)
  22. where tftph pointer is not moving from one iteration to another, causing
  23. to serve same packet again. Luckily, double serving didn't happen due to
  24. data->block++ during the first iteration.
  25. Fixes: CID 73624, CID 96690
  26. Signed-off-by: Alexey Makhalov <amakhalov@vmware.com>
  27. Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
  28. Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com>
  29. ---
  30. grub-core/net/tftp.c | 168 ++++++++++++++-----------------------------
  31. 1 file changed, 53 insertions(+), 115 deletions(-)
  32. diff --git a/grub-core/net/tftp.c b/grub-core/net/tftp.c
  33. index 7d90bf66e..b4297bc8d 100644
  34. --- a/grub-core/net/tftp.c
  35. +++ b/grub-core/net/tftp.c
  36. @@ -25,7 +25,6 @@
  37. #include <grub/mm.h>
  38. #include <grub/dl.h>
  39. #include <grub/file.h>
  40. -#include <grub/priority_queue.h>
  41. #include <grub/i18n.h>
  42. GRUB_MOD_LICENSE ("GPLv3+");
  43. @@ -106,31 +105,8 @@ typedef struct tftp_data
  44. int have_oack;
  45. struct grub_error_saved save_err;
  46. grub_net_udp_socket_t sock;
  47. - grub_priority_queue_t pq;
  48. } *tftp_data_t;
  49. -static int
  50. -cmp_block (grub_uint16_t a, grub_uint16_t b)
  51. -{
  52. - grub_int16_t i = (grub_int16_t) (a - b);
  53. - if (i > 0)
  54. - return +1;
  55. - if (i < 0)
  56. - return -1;
  57. - return 0;
  58. -}
  59. -
  60. -static int
  61. -cmp (const void *a__, const void *b__)
  62. -{
  63. - struct grub_net_buff *a_ = *(struct grub_net_buff **) a__;
  64. - struct grub_net_buff *b_ = *(struct grub_net_buff **) b__;
  65. - struct tftphdr *a = (struct tftphdr *) a_->data;
  66. - struct tftphdr *b = (struct tftphdr *) b_->data;
  67. - /* We want the first elements to be on top. */
  68. - return -cmp_block (grub_be_to_cpu16 (a->u.data.block), grub_be_to_cpu16 (b->u.data.block));
  69. -}
  70. -
  71. static grub_err_t
  72. ack (tftp_data_t data, grub_uint64_t block)
  73. {
  74. @@ -207,73 +183,60 @@ tftp_receive (grub_net_udp_socket_t sock __attribute__ ((unused)),
  75. return GRUB_ERR_NONE;
  76. }
  77. - err = grub_priority_queue_push (data->pq, &nb);
  78. - if (err)
  79. - return err;
  80. -
  81. - {
  82. - struct grub_net_buff **nb_top_p, *nb_top;
  83. - while (1)
  84. - {
  85. - nb_top_p = grub_priority_queue_top (data->pq);
  86. - if (!nb_top_p)
  87. - return GRUB_ERR_NONE;
  88. - nb_top = *nb_top_p;
  89. - tftph = (struct tftphdr *) nb_top->data;
  90. - if (cmp_block (grub_be_to_cpu16 (tftph->u.data.block), data->block + 1) >= 0)
  91. - break;
  92. - ack (data, grub_be_to_cpu16 (tftph->u.data.block));
  93. - grub_netbuff_free (nb_top);
  94. - grub_priority_queue_pop (data->pq);
  95. - }
  96. - while (cmp_block (grub_be_to_cpu16 (tftph->u.data.block), data->block + 1) == 0)
  97. - {
  98. - unsigned size;
  99. -
  100. - grub_priority_queue_pop (data->pq);
  101. -
  102. - if (file->device->net->packs.count < 50)
  103. + /* Ack old/retransmitted block. */
  104. + if (grub_be_to_cpu16 (tftph->u.data.block) < data->block + 1)
  105. + ack (data, grub_be_to_cpu16 (tftph->u.data.block));
  106. + /* Ignore unexpected block. */
  107. + else if (grub_be_to_cpu16 (tftph->u.data.block) > data->block + 1)
  108. + grub_dprintf ("tftp", "TFTP unexpected block # %d\n", tftph->u.data.block);
  109. + else
  110. + {
  111. + unsigned size;
  112. +
  113. + if (file->device->net->packs.count < 50)
  114. + {
  115. err = ack (data, data->block + 1);
  116. - else
  117. - {
  118. - file->device->net->stall = 1;
  119. - err = 0;
  120. - }
  121. - if (err)
  122. - return err;
  123. -
  124. - err = grub_netbuff_pull (nb_top, sizeof (tftph->opcode) +
  125. - sizeof (tftph->u.data.block));
  126. - if (err)
  127. - return err;
  128. - size = nb_top->tail - nb_top->data;
  129. -
  130. - data->block++;
  131. - if (size < data->block_size)
  132. - {
  133. - if (data->ack_sent < data->block)
  134. - ack (data, data->block);
  135. - file->device->net->eof = 1;
  136. - file->device->net->stall = 1;
  137. - grub_net_udp_close (data->sock);
  138. - data->sock = NULL;
  139. - }
  140. - /* Prevent garbage in broken cards. Is it still necessary
  141. - given that IP implementation has been fixed?
  142. - */
  143. - if (size > data->block_size)
  144. - {
  145. - err = grub_netbuff_unput (nb_top, size - data->block_size);
  146. - if (err)
  147. - return err;
  148. - }
  149. - /* If there is data, puts packet in socket list. */
  150. - if ((nb_top->tail - nb_top->data) > 0)
  151. - grub_net_put_packet (&file->device->net->packs, nb_top);
  152. - else
  153. - grub_netbuff_free (nb_top);
  154. - }
  155. - }
  156. + if (err)
  157. + return err;
  158. + }
  159. + else
  160. + file->device->net->stall = 1;
  161. +
  162. + err = grub_netbuff_pull (nb, sizeof (tftph->opcode) +
  163. + sizeof (tftph->u.data.block));
  164. + if (err)
  165. + return err;
  166. + size = nb->tail - nb->data;
  167. +
  168. + data->block++;
  169. + if (size < data->block_size)
  170. + {
  171. + if (data->ack_sent < data->block)
  172. + ack (data, data->block);
  173. + file->device->net->eof = 1;
  174. + file->device->net->stall = 1;
  175. + grub_net_udp_close (data->sock);
  176. + data->sock = NULL;
  177. + }
  178. + /*
  179. + * Prevent garbage in broken cards. Is it still necessary
  180. + * given that IP implementation has been fixed?
  181. + */
  182. + if (size > data->block_size)
  183. + {
  184. + err = grub_netbuff_unput (nb, size - data->block_size);
  185. + if (err)
  186. + return err;
  187. + }
  188. + /* If there is data, puts packet in socket list. */
  189. + if ((nb->tail - nb->data) > 0)
  190. + {
  191. + grub_net_put_packet (&file->device->net->packs, nb);
  192. + /* Do not free nb. */
  193. + return GRUB_ERR_NONE;
  194. + }
  195. + }
  196. + grub_netbuff_free (nb);
  197. return GRUB_ERR_NONE;
  198. case TFTP_ERROR:
  199. data->have_oack = 1;
  200. @@ -287,19 +250,6 @@ tftp_receive (grub_net_udp_socket_t sock __attribute__ ((unused)),
  201. }
  202. }
  203. -static void
  204. -destroy_pq (tftp_data_t data)
  205. -{
  206. - struct grub_net_buff **nb_p;
  207. - while ((nb_p = grub_priority_queue_top (data->pq)))
  208. - {
  209. - grub_netbuff_free (*nb_p);
  210. - grub_priority_queue_pop (data->pq);
  211. - }
  212. -
  213. - grub_priority_queue_destroy (data->pq);
  214. -}
  215. -
  216. static grub_err_t
  217. tftp_open (struct grub_file *file, const char *filename)
  218. {
  219. @@ -372,17 +322,9 @@ tftp_open (struct grub_file *file, const char *filename)
  220. file->not_easily_seekable = 1;
  221. file->data = data;
  222. - data->pq = grub_priority_queue_new (sizeof (struct grub_net_buff *), cmp);
  223. - if (!data->pq)
  224. - {
  225. - grub_free (data);
  226. - return grub_errno;
  227. - }
  228. -
  229. err = grub_net_resolve_address (file->device->net->server, &addr);
  230. if (err)
  231. {
  232. - destroy_pq (data);
  233. grub_free (data);
  234. return err;
  235. }
  236. @@ -392,7 +334,6 @@ tftp_open (struct grub_file *file, const char *filename)
  237. file);
  238. if (!data->sock)
  239. {
  240. - destroy_pq (data);
  241. grub_free (data);
  242. return grub_errno;
  243. }
  244. @@ -406,7 +347,6 @@ tftp_open (struct grub_file *file, const char *filename)
  245. if (err)
  246. {
  247. grub_net_udp_close (data->sock);
  248. - destroy_pq (data);
  249. grub_free (data);
  250. return err;
  251. }
  252. @@ -423,7 +363,6 @@ tftp_open (struct grub_file *file, const char *filename)
  253. if (grub_errno)
  254. {
  255. grub_net_udp_close (data->sock);
  256. - destroy_pq (data);
  257. grub_free (data);
  258. return grub_errno;
  259. }
  260. @@ -466,7 +405,6 @@ tftp_close (struct grub_file *file)
  261. grub_print_error ();
  262. grub_net_udp_close (data->sock);
  263. }
  264. - destroy_pq (data);
  265. grub_free (data);
  266. return GRUB_ERR_NONE;
  267. }
  268. --
  269. 2.26.2