VLogError(): Avoid NULL pointer dereferences in (V)Sprint calls

Backport of upstream fix:

VLogError() calculates the size of format strings by using calls to
SPrint and VSPrint with a StrSize of 0 and NULL for an output
buffer. Unfortunately, this is an incorrect usage of (V)Sprint. A
StrSize of "0" is special-cased to mean "there is no limit". So, we
end up writing our string to address 0x0. This was discovered because
it causes a crash on ARM where, unlike x86, it does not necessarily
have memory mapped at 0x0.

Avoid the (V)Sprint calls altogether by using (V)PoolPrint, which
handles the size calculation and allocation for us.

Signed-off-by: Peter Jones <pjones@redhat.com>
Fixes: 25f6fd08cd26 ("try to show errors more usefully.")
[dannf: commit message ]
Signed-off-by: dann frazier <dann.frazier@canonical.com>

diff --git a/debian/changelog b/debian/changelog
index 45eadbffbbefd7c69db6b79bfeec30eeb8cd46fd..95d6dbc5347d6ce43d2f87dba468f674ab4c5b61 100644 (file)
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+shim (15+1533136590.3beb971-7) UNRELEASED; urgency=medium
+
+  [ Steve McIntyre ]
+  * Backport needed crash fixes:
+    + VLogError(): Avoid NULL pointer dereferences in (V)Sprint calls
+
+ -- Steve McIntyre <93sam@debian.org>  Fri, 03 May 2019 01:39:34 +0100
+
 shim (15+1533136590.3beb971-6) unstable; urgency=medium
 
   [ Steve McIntyre ]

diff --git a/debian/patches/avoid_null_vsprint.patch b/debian/patches/avoid_null_vsprint.patch
new file mode 100644 (file)
index 0000000..cb056d6
--- /dev/null
+++ b/debian/patches/avoid_null_vsprint.patch
@@ -0,0 +1,59 @@
+commit 20e731f423a438f53738de73af9ef3d67c4cba2f
+Author: Peter Jones <pjones@redhat.com>
+Date:   Tue Feb 12 18:04:49 2019 -0500
+
+    VLogError(): Avoid NULL pointer dereferences in (V)Sprint calls
+    
+    VLogError() calculates the size of format strings by using calls to
+    SPrint and VSPrint with a StrSize of 0 and NULL for an output buffer.
+    Unfortunately, this is an incorrect usage of (V)Sprint. A StrSize
+    of "0" is special-cased to mean "there is no limit". So, we end up
+    writing our string to address 0x0. This was discovered because it
+    causes a crash on ARM where, unlike x86, it does not necessarily
+    have memory mapped at 0x0.
+    
+    Avoid the (V)Sprint calls altogether by using (V)PoolPrint, which
+    handles the size calculation and allocation for us.
+    
+    Signed-off-by: Peter Jones <pjones@redhat.com>
+    Fixes: 25f6fd08cd26 ("try to show errors more usefully.")
+    [dannf: commit message ]
+    Signed-off-by: dann frazier <dann.frazier@canonical.com>
+
+diff --git a/errlog.c b/errlog.c
+index 18be482..eebb266 100644
+--- a/errlog.c
++++ b/errlog.c
+@@ -14,29 +14,20 @@ EFI_STATUS
+ VLogError(const char *file, int line, const char *func, CHAR16 *fmt, va_list args)
+ {
+       va_list args2;
+-      UINTN size = 0, size2;
+       CHAR16 **newerrs;
+ 
+-      size = SPrint(NULL, 0, L"%a:%d %a() ", file, line, func);
+-      va_copy(args2, args);
+-      size2 = VSPrint(NULL, 0, fmt, args2);
+-      va_end(args2);
+-
+       newerrs = ReallocatePool(errs, (nerrs + 1) * sizeof(*errs),
+                                      (nerrs + 3) * sizeof(*errs));
+       if (!newerrs)
+               return EFI_OUT_OF_RESOURCES;
+ 
+-      newerrs[nerrs] = AllocatePool(size*2+2);
++      newerrs[nerrs] = PoolPrint(L"%a:%d %a() ", file, line, func);
+       if (!newerrs[nerrs])
+               return EFI_OUT_OF_RESOURCES;
+-      newerrs[nerrs+1] = AllocatePool(size2*2+2);
++      va_copy(args2, args);
++      newerrs[nerrs+1] = VPoolPrint(fmt, args2);
+       if (!newerrs[nerrs+1])
+               return EFI_OUT_OF_RESOURCES;
+-
+-      SPrint(newerrs[nerrs], size*2+2, L"%a:%d %a() ", file, line, func);
+-      va_copy(args2, args);
+-      VSPrint(newerrs[nerrs+1], size2*2+2, fmt, args2);
+       va_end(args2);
+ 
+       nerrs += 2;

diff --git a/debian/patches/series b/debian/patches/series
index 01e6063f932e9140be221beb564a9586d026b37f..9cae2bbf53465b6adca5516cfd167cf02f17b6f0 100644 (file)
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1,2 +1,3 @@
 fixup_git.patch
 uname.patch
+avoid_null_vsprint.patch