Commit 2cf10e0e authored by Michał Adamski's avatar Michał Adamski Committed by Michał Adamski

main/grub: boothole patches

parent a60dba7e
Pipeline #33750 failed with stages
in 4 minutes and 2 seconds
From 6d5cd112c65caf7c46518a79a27fe7930ec4714e Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
Date: Wed, 15 Apr 2020 15:45:02 -0400
Subject: yylex: Make lexer fatal errors actually be fatal
When presented with a command that can't be tokenized to anything
smaller than YYLMAX characters, the parser calls YY_FATAL_ERROR(errmsg),
expecting that will stop further processing, as such:
#define YY_DO_BEFORE_ACTION \
yyg->yytext_ptr = yy_bp; \
yyleng = (int) (yy_cp - yy_bp); \
yyg->yy_hold_char = *yy_cp; \
*yy_cp = '\0'; \
if ( yyleng >= YYLMAX ) \
YY_FATAL_ERROR( "token too large, exceeds YYLMAX" ); \
yy_flex_strncpy( yytext, yyg->yytext_ptr, yyleng + 1 , yyscanner); \
yyg->yy_c_buf_p = yy_cp;
The code flex generates expects that YY_FATAL_ERROR() will either return
for it or do some form of longjmp(), or handle the error in some way at
least, and so the strncpy() call isn't in an "else" clause, and thus if
YY_FATAL_ERROR() is *not* actually fatal, it does the call with the
questionable limit, and predictable results ensue.
Unfortunately, our implementation of YY_FATAL_ERROR() is:
#define YY_FATAL_ERROR(msg) \
do { \
grub_printf (_("fatal error: %s\n"), _(msg)); \
} while (0)
The same pattern exists in yyless(), and similar problems exist in users
of YY_INPUT(), several places in the main parsing loop,
yy_get_next_buffer(), yy_load_buffer_state(), yyensure_buffer_stack,
yy_scan_buffer(), etc.
All of these callers expect YY_FATAL_ERROR() to actually be fatal, and
the things they do if it returns after calling it are wildly unsafe.
Fixes: CVE-2020-10713
Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
grub-core/script/yylex.l | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/grub-core/script/yylex.l b/grub-core/script/yylex.l
index 7b44c37b7..b7203c823 100644
--- a/grub-core/script/yylex.l
+++ b/grub-core/script/yylex.l
@@ -37,11 +37,11 @@
/*
* As we don't have access to yyscanner, we cannot do much except to
- * print the fatal error.
+ * print the fatal error and exit.
*/
#define YY_FATAL_ERROR(msg) \
do { \
- grub_printf (_("fatal error: %s\n"), _(msg)); \
+ grub_fatal (_("fatal error: %s\n"), _(msg));\
} while (0)
#define COPY(str, hint) \
From 79fe92c7e4ae4b6fd13a3db8f1a2ae7729e0e265 Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
Date: Mon, 15 Jun 2020 10:58:42 -0400
Subject: safemath: Add some arithmetic primitives that check for overflow
This adds a new header, include/grub/safemath.h, that includes easy to
use wrappers for __builtin_{add,sub,mul}_overflow() declared like:
bool OP(a, b, res)
where OP is grub_add, grub_sub or grub_mul. OP() returns true in the
case where the operation would overflow and res is not modified.
Otherwise, false is returned and the operation is executed.
These arithmetic primitives require newer compiler versions. So, bump
these requirements in the INSTALL file too.
Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
INSTALL | 22 ++--------------------
include/grub/compiler.h | 8 ++++++++
include/grub/safemath.h | 37 +++++++++++++++++++++++++++++++++++++
3 files changed, 47 insertions(+), 20 deletions(-)
create mode 100644 include/grub/safemath.h
diff --git a/INSTALL b/INSTALL
index 342c158e9..991479b52 100644
--- a/INSTALL
+++ b/INSTALL
@@ -11,27 +11,9 @@ GRUB depends on some software packages installed into your system. If
you don't have any of them, please obtain and install them before
configuring the GRUB.
-* GCC 4.1.3 or later
- Note: older versions may work but support is limited
-
- Experimental support for clang 3.3 or later (results in much bigger binaries)
+* GCC 5.1.0 or later
+ Experimental support for clang 3.8.0 or later (results in much bigger binaries)
for i386, x86_64, arm (including thumb), arm64, mips(el), powerpc, sparc64
- Note: clang 3.2 or later works for i386 and x86_64 targets but results in
- much bigger binaries.
- earlier versions not tested
- Note: clang 3.2 or later works for arm
- earlier versions not tested
- Note: clang on arm64 is not supported due to
- https://llvm.org/bugs/show_bug.cgi?id=26030
- Note: clang 3.3 or later works for mips(el)
- earlier versions fail to generate .reginfo and hence gprel relocations
- fail.
- Note: clang 3.2 or later works for powerpc
- earlier versions not tested
- Note: clang 3.5 or later works for sparc64
- earlier versions return "error: unable to interface with target machine"
- Note: clang has no support for ia64 and hence you can't compile GRUB
- for ia64 with clang
* GNU Make
* GNU Bison 2.3 or later
* GNU gettext 0.17 or later
diff --git a/include/grub/compiler.h b/include/grub/compiler.h
index c9e1d7a73..8f3be3ae7 100644
--- a/include/grub/compiler.h
+++ b/include/grub/compiler.h
@@ -48,4 +48,12 @@
# define WARN_UNUSED_RESULT
#endif
+#if defined(__clang__) && defined(__clang_major__) && defined(__clang_minor__)
+# define CLANG_PREREQ(maj,min) \
+ ((__clang_major__ > (maj)) || \
+ (__clang_major__ == (maj) && __clang_minor__ >= (min)))
+#else
+# define CLANG_PREREQ(maj,min) 0
+#endif
+
#endif /* ! GRUB_COMPILER_HEADER */
diff --git a/include/grub/safemath.h b/include/grub/safemath.h
new file mode 100644
index 000000000..c17b89bba
--- /dev/null
+++ b/include/grub/safemath.h
@@ -0,0 +1,37 @@
+/*
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2020 Free Software Foundation, Inc.
+ *
+ * GRUB is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GRUB is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Arithmetic operations that protect against overflow.
+ */
+
+#ifndef GRUB_SAFEMATH_H
+#define GRUB_SAFEMATH_H 1
+
+#include <grub/compiler.h>
+
+/* These appear in gcc 5.1 and clang 3.8. */
+#if GNUC_PREREQ(5, 1) || CLANG_PREREQ(3, 8)
+
+#define grub_add(a, b, res) __builtin_add_overflow(a, b, res)
+#define grub_sub(a, b, res) __builtin_sub_overflow(a, b, res)
+#define grub_mul(a, b, res) __builtin_mul_overflow(a, b, res)
+
+#else
+#error gcc 5.1 or newer or clang 3.8 or newer is required
+#endif
+
+#endif /* GRUB_SAFEMATH_H */
From 9e7a74bc530afaad40667b397218615fde3e44ef Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
Date: Mon, 15 Jun 2020 12:15:29 -0400
Subject: calloc: Make sure we always have an overflow-checking calloc()
available
This tries to make sure that everywhere in this source tree, we always have
an appropriate version of calloc() (i.e. grub_calloc(), xcalloc(), etc.)
available, and that they all safely check for overflow and return NULL when
it would occur.
Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
grub-core/kern/emu/misc.c | 12 +++++++++
grub-core/kern/emu/mm.c | 10 ++++++++
grub-core/kern/mm.c | 40 ++++++++++++++++++++++++++++++
grub-core/lib/libgcrypt_wrap/mem.c | 11 ++++++--
grub-core/lib/posix_wrap/stdlib.h | 8 +++++-
include/grub/emu/misc.h | 1 +
include/grub/mm.h | 6 +++++
7 files changed, 85 insertions(+), 3 deletions(-)
diff --git a/grub-core/kern/emu/misc.c b/grub-core/kern/emu/misc.c
index 65db79baa..dfd8a8ec4 100644
--- a/grub-core/kern/emu/misc.c
+++ b/grub-core/kern/emu/misc.c
@@ -85,6 +85,18 @@ grub_util_error (const char *fmt, ...)
exit (1);
}
+void *
+xcalloc (grub_size_t nmemb, grub_size_t size)
+{
+ void *p;
+
+ p = calloc (nmemb, size);
+ if (!p)
+ grub_util_error ("%s", _("out of memory"));
+
+ return p;
+}
+
void *
xmalloc (grub_size_t size)
{
diff --git a/grub-core/kern/emu/mm.c b/grub-core/kern/emu/mm.c
index f262e95e3..145b01d37 100644
--- a/grub-core/kern/emu/mm.c
+++ b/grub-core/kern/emu/mm.c
@@ -25,6 +25,16 @@
#include <string.h>
#include <grub/i18n.h>
+void *
+grub_calloc (grub_size_t nmemb, grub_size_t size)
+{
+ void *ret;
+ ret = calloc (nmemb, size);
+ if (!ret)
+ grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+ return ret;
+}
+
void *
grub_malloc (grub_size_t size)
{
diff --git a/grub-core/kern/mm.c b/grub-core/kern/mm.c
index ee88ff611..f2822a836 100644
--- a/grub-core/kern/mm.c
+++ b/grub-core/kern/mm.c
@@ -67,8 +67,10 @@
#include <grub/dl.h>
#include <grub/i18n.h>
#include <grub/mm_private.h>
+#include <grub/safemath.h>
#ifdef MM_DEBUG
+# undef grub_calloc
# undef grub_malloc
# undef grub_zalloc
# undef grub_realloc
@@ -375,6 +377,30 @@ grub_memalign (grub_size_t align, grub_size_t size)
return 0;
}
+/*
+ * Allocate NMEMB instances of SIZE bytes and return the pointer, or error on
+ * integer overflow.
+ */
+void *
+grub_calloc (grub_size_t nmemb, grub_size_t size)
+{
+ void *ret;
+ grub_size_t sz = 0;
+
+ if (grub_mul (nmemb, size, &sz))
+ {
+ grub_error (GRUB_ERR_OUT_OF_RANGE, N_("overflow is detected"));
+ return NULL;
+ }
+
+ ret = grub_memalign (0, sz);
+ if (!ret)
+ return NULL;
+
+ grub_memset (ret, 0, sz);
+ return ret;
+}
+
/* Allocate SIZE bytes and return the pointer. */
void *
grub_malloc (grub_size_t size)
@@ -561,6 +587,20 @@ grub_mm_dump (unsigned lineno)
grub_printf ("\n");
}
+void *
+grub_debug_calloc (const char *file, int line, grub_size_t nmemb, grub_size_t size)
+{
+ void *ptr;
+
+ if (grub_mm_debug)
+ grub_printf ("%s:%d: calloc (0x%" PRIxGRUB_SIZE ", 0x%" PRIxGRUB_SIZE ") = ",
+ file, line, size);
+ ptr = grub_calloc (nmemb, size);
+ if (grub_mm_debug)
+ grub_printf ("%p\n", ptr);
+ return ptr;
+}
+
void *
grub_debug_malloc (const char *file, int line, grub_size_t size)
{
diff --git a/grub-core/lib/libgcrypt_wrap/mem.c b/grub-core/lib/libgcrypt_wrap/mem.c
index beeb661a3..74c6eafe5 100644
--- a/grub-core/lib/libgcrypt_wrap/mem.c
+++ b/grub-core/lib/libgcrypt_wrap/mem.c
@@ -4,6 +4,7 @@
#include <grub/crypto.h>
#include <grub/dl.h>
#include <grub/env.h>
+#include <grub/safemath.h>
GRUB_MOD_LICENSE ("GPLv3+");
@@ -36,7 +37,10 @@ void *
gcry_xcalloc (size_t n, size_t m)
{
void *ret;
- ret = grub_zalloc (n * m);
+ size_t sz;
+ if (grub_mul (n, m, &sz))
+ grub_fatal ("gcry_xcalloc would overflow");
+ ret = grub_zalloc (sz);
if (!ret)
grub_fatal ("gcry_xcalloc failed");
return ret;
@@ -56,7 +60,10 @@ void *
gcry_xcalloc_secure (size_t n, size_t m)
{
void *ret;
- ret = grub_zalloc (n * m);
+ size_t sz;
+ if (grub_mul (n, m, &sz))
+ grub_fatal ("gcry_xcalloc would overflow");
+ ret = grub_zalloc (sz);
if (!ret)
grub_fatal ("gcry_xcalloc failed");
return ret;
diff --git a/grub-core/lib/posix_wrap/stdlib.h b/grub-core/lib/posix_wrap/stdlib.h
index 3b46f47ff..7a8d385e9 100644
--- a/grub-core/lib/posix_wrap/stdlib.h
+++ b/grub-core/lib/posix_wrap/stdlib.h
@@ -21,6 +21,7 @@
#include <grub/mm.h>
#include <grub/misc.h>
+#include <grub/safemath.h>
static inline void
free (void *ptr)
@@ -37,7 +38,12 @@ malloc (grub_size_t size)
static inline void *
calloc (grub_size_t size, grub_size_t nelem)
{
- return grub_zalloc (size * nelem);
+ grub_size_t sz;
+
+ if (grub_mul (size, nelem, &sz))
+ return NULL;
+
+ return grub_zalloc (sz);
}
static inline void *
diff --git a/include/grub/emu/misc.h b/include/grub/emu/misc.h
index ce464cfd0..ff9c48a64 100644
--- a/include/grub/emu/misc.h
+++ b/include/grub/emu/misc.h
@@ -47,6 +47,7 @@ grub_util_device_is_mapped (const char *dev);
#define GRUB_HOST_PRIuLONG_LONG "llu"
#define GRUB_HOST_PRIxLONG_LONG "llx"
+void * EXPORT_FUNC(xcalloc) (grub_size_t nmemb, grub_size_t size) WARN_UNUSED_RESULT;
void * EXPORT_FUNC(xmalloc) (grub_size_t size) WARN_UNUSED_RESULT;
void * EXPORT_FUNC(xrealloc) (void *ptr, grub_size_t size) WARN_UNUSED_RESULT;
char * EXPORT_FUNC(xstrdup) (const char *str) WARN_UNUSED_RESULT;
diff --git a/include/grub/mm.h b/include/grub/mm.h
index 28e2e53eb..9c38dd3ca 100644
--- a/include/grub/mm.h
+++ b/include/grub/mm.h
@@ -29,6 +29,7 @@
#endif
void grub_mm_init_region (void *addr, grub_size_t size);
+void *EXPORT_FUNC(grub_calloc) (grub_size_t nmemb, grub_size_t size);
void *EXPORT_FUNC(grub_malloc) (grub_size_t size);
void *EXPORT_FUNC(grub_zalloc) (grub_size_t size);
void EXPORT_FUNC(grub_free) (void *ptr);
@@ -48,6 +49,9 @@ extern int EXPORT_VAR(grub_mm_debug);
void grub_mm_dump_free (void);
void grub_mm_dump (unsigned lineno);
+#define grub_calloc(nmemb, size) \
+ grub_debug_calloc (GRUB_FILE, __LINE__, nmemb, size)
+
#define grub_malloc(size) \
grub_debug_malloc (GRUB_FILE, __LINE__, size)
@@ -63,6 +67,8 @@ void grub_mm_dump (unsigned lineno);
#define grub_free(ptr) \
grub_debug_free (GRUB_FILE, __LINE__, ptr)
+void *EXPORT_FUNC(grub_debug_calloc) (const char *file, int line,
+ grub_size_t nmemb, grub_size_t size);
void *EXPORT_FUNC(grub_debug_malloc) (const char *file, int line,
grub_size_t size);
void *EXPORT_FUNC(grub_debug_zalloc) (const char *file, int line,
This diff is collapsed.
From ae8703a3dd74ec31c8f37135fc32315f926812b2 Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
Date: Sat, 4 Jul 2020 12:25:09 -0400
Subject: iso9660: Don't leak memory on realloc() failures
Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
grub-core/fs/iso9660.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/grub-core/fs/iso9660.c b/grub-core/fs/iso9660.c
index 7ba5b300b..5ec4433b8 100644
--- a/grub-core/fs/iso9660.c
+++ b/grub-core/fs/iso9660.c
@@ -533,14 +533,20 @@ add_part (struct iterate_dir_ctx *ctx,
{
int size = ctx->symlink ? grub_strlen (ctx->symlink) : 0;
grub_size_t sz;
+ char *new;
if (grub_add (size, len2, &sz) ||
grub_add (sz, 1, &sz))
return;
- ctx->symlink = grub_realloc (ctx->symlink, sz);
- if (! ctx->symlink)
- return;
+ new = grub_realloc (ctx->symlink, sz);
+ if (!new)
+ {
+ grub_free (ctx->symlink);
+ ctx->symlink = NULL;
+ return;
+ }
+ ctx->symlink = new;
grub_memcpy (ctx->symlink + size, part, len2);
ctx->symlink[size + len2] = 0;
@@ -634,7 +640,12 @@ susp_iterate_dir (struct grub_iso9660_susp_entry *entry,
is the length. Both are part of the `Component
Record'. */
if (ctx->symlink && !ctx->was_continue)
- add_part (ctx, "/", 1);
+ {
+ add_part (ctx, "/", 1);
+ if (grub_errno)
+ return grub_errno;
+ }
+
add_part (ctx, (char *) &entry->data[pos + 2],
entry->data[pos + 1]);
ctx->was_continue = (entry->data[pos] & 1);
@@ -653,6 +664,11 @@ susp_iterate_dir (struct grub_iso9660_susp_entry *entry,
add_part (ctx, "/", 1);
break;
}
+
+ /* Check if grub_realloc() failed in add_part(). */
+ if (grub_errno)
+ return grub_errno;
+
/* In pos + 1 the length of the `Component Record' is
stored. */
pos += entry->data[pos + 1] + 2;
From aa83281cfdc2b7a508b8fa21018a3b31e6f0440d Mon Sep 17 00:00:00 2001
From: Daniel Kiper <daniel.kiper@oracle.com>
Date: Tue, 7 Jul 2020 15:36:26 +0200
Subject: font: Do not load more than one NAME section
The GRUB font file can have one NAME section only. Though if somebody
crafts a broken font file with many NAME sections and loads it then the
GRUB leaks memory. So, prevent against that by loading first NAME
section and failing in controlled way on following one.
Reported-by: Chris Coulson <chris.coulson@canonical.com>
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Jan Setje-Eilers <jan.setjeeilers@oracle.com>
---
grub-core/font/font.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/grub-core/font/font.c b/grub-core/font/font.c
index 5edb477ac..d09bb38d8 100644
--- a/grub-core/font/font.c
+++ b/grub-core/font/font.c
@@ -532,6 +532,12 @@ grub_font_load (const char *filename)
if (grub_memcmp (section.name, FONT_FORMAT_SECTION_NAMES_FONT_NAME,
sizeof (FONT_FORMAT_SECTION_NAMES_FONT_NAME) - 1) == 0)
{
+ if (font->name != NULL)
+ {
+ grub_error (GRUB_ERR_BAD_FONT, "invalid font file: too many NAME sections");
+ goto fail;
+ }
+
font->name = read_section_as_string (&section);
if (!font->name)
goto fail;
From 72c7f2afab0e9ea5d17b35ea4ff74221e1bcfbfc Mon Sep 17 00:00:00 2001
From: Alexey Makhalov <amakhalov@vmware.com>
Date: Wed, 8 Jul 2020 20:41:56 +0000
Subject: gfxmenu: Fix double free in load_image()
self->bitmap should be zeroed after free. Otherwise, there is a chance
to double free (USE_AFTER_FREE) it later in rescale_image().
Fixes: CID 292472
Signed-off-by: Alexey Makhalov <amakhalov@vmware.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
grub-core/gfxmenu/gui_image.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/grub-core/gfxmenu/gui_image.c b/grub-core/gfxmenu/gui_image.c
index 29784ed2d..6b2e976f1 100644
--- a/grub-core/gfxmenu/gui_image.c
+++ b/grub-core/gfxmenu/gui_image.c
@@ -195,7 +195,10 @@ load_image (grub_gui_image_t self, const char *path)
return grub_errno;
if (self->bitmap && (self->bitmap != self->raw_bitmap))
- grub_video_bitmap_destroy (self->bitmap);
+ {
+ grub_video_bitmap_destroy (self->bitmap);
+ self->bitmap = 0;
+ }
if (self->raw_bitmap)
grub_video_bitmap_destroy (self->raw_bitmap);
From c8aef3a92c63add6c4c4b65bbf17c2121d97a688 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Thu, 9 Jul 2020 03:05:23 +0000
Subject: lzma: Make sure we don't dereference past array
The two dimensional array p->posSlotEncoder[4][64] is being dereferenced
using the GetLenToPosState() macro which checks if len is less than 5,
and if so subtracts 2 from it. If len = 0, that is 0 - 2 = 4294967294.
Obviously we don't want to dereference that far out so we check if the
position found is greater or equal kNumLenToPosStates (4) and bail out.
N.B.: Upstream LZMA 18.05 and later has this function completely rewritten
without any history.
Fixes: CID 51526
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
grub-core/lib/LzmaEnc.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/grub-core/lib/LzmaEnc.c b/grub-core/lib/LzmaEnc.c
index f2ec04a8c..753e56a95 100644
--- a/grub-core/lib/LzmaEnc.c
+++ b/grub-core/lib/LzmaEnc.c
@@ -1877,13 +1877,19 @@ static SRes LzmaEnc_CodeOneBlock(CLzmaEnc *p, Bool useLimits, UInt32 maxPackSize
}
else
{
- UInt32 posSlot;
+ UInt32 posSlot, lenToPosState;
RangeEnc_EncodeBit(&p->rc, &p->isRep[p->state], 0);
p->state = kMatchNextStates[p->state];
LenEnc_Encode2(&p->lenEnc, &p->rc, len - LZMA_MATCH_LEN_MIN, posState, !p->fastMode, p->ProbPrices);
pos -= LZMA_NUM_REPS;
GetPosSlot(pos, posSlot);
- RcTree_Encode(&p->rc, p->posSlotEncoder[GetLenToPosState(len)], kNumPosSlotBits, posSlot);
+ lenToPosState = GetLenToPosState(len);
+ if (lenToPosState >= kNumLenToPosStates)
+ {
+ p->result = SZ_ERROR_DATA;
+ return CheckErrors(p);
+ }
+ RcTree_Encode(&p->rc, p->posSlotEncoder[lenToPosState], kNumPosSlotBits, posSlot);
if (posSlot >= kStartPosModelIndex)
{
From 80e584d2d9a255c7626bac198fb7e335a63a1f51 Mon Sep 17 00:00:00 2001
From: Alexey Makhalov <amakhalov@vmware.com>
Date: Thu, 9 Jul 2020 08:10:40 +0000
Subject: tftp: Do not use priority queue
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>
---
grub-core/net/tftp.c | 171 ++++++++++++++-----------------------------
1 file changed, 53 insertions(+), 118 deletions(-)
diff --git a/grub-core/net/tftp.c b/grub-core/net/tftp.c
index a0817a075..e6566fa17 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;