1: refine commit 9143a6c55ef7 for the 64-bit case 2: pull out constant tables 3: fix system halt at boot kernel on x86_64 Only patch 1 is strictly meant to be considered for 4.13, albeit patch 3 fixes a similar problem (but none which would have been reported so far). Patches 2 and 3 are ports of (a couple of years old) Linux commits. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
On 05.12.19 08:26, Jan Beulich wrote: > 1: refine commit 9143a6c55ef7 for the 64-bit case > 2: pull out constant tables > 3: fix system halt at boot kernel on x86_64 > > Only patch 1 is strictly meant to be considered for 4.13, albeit > patch 3 fixes a similar problem (but none which would have been > reported so far). Patches 2 and 3 are ports of (a couple of > years old) Linux commits. > > Jan Release-acked-by: Juergen Gross <jgross@suse.com> Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
On 05/12/2019 07:26, Jan Beulich wrote: > 1: refine commit 9143a6c55ef7 for the 64-bit case > 2: pull out constant tables > 3: fix system halt at boot kernel on x86_64 > > Only patch 1 is strictly meant to be considered for 4.13, albeit > patch 3 fixes a similar problem (but none which would have been > reported so far). Patches 2 and 3 are ports of (a couple of > years old) Linux commits. Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> I'd chuck them all in for 4.13. The moment we decide to leave patch 3 out, someone is going to stumble over the issue. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
I clearly went too far there: While the LZ4_WILDCOPY() instances indeed need prior guarding, LZ4_SECURECOPY() needs this only in the 32-bit case (where it simply aliases LZ4_WILDCOPY()). "cpy" can validly point (slightly) below "op" in these cases, due to cpy = op + length - (STEPSIZE - 4); where length can be as low as 0 and STEPSIZE is 8. However, instead of removing the check via "#if !LZ4_ARCH64", refine it such that it would also properly work in the 64-bit case, aborting decompression instead of continuing on bogus input. Reported-by: Mark Pryor <pryorm09@gmail.com> Reported-by: Jeremi Piotrowski <jeremi.piotrowski@gmail.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Tested-by: Mark Pryor <pryorm09@gmail.com> --- a/xen/common/lz4/decompress.c +++ b/xen/common/lz4/decompress.c @@ -147,7 +147,7 @@ static int INIT lz4_uncompress(const uns goto _output_error; continue; } - if (unlikely((unsigned long)cpy < (unsigned long)op)) + if (unlikely((unsigned long)cpy < (unsigned long)op - (STEPSIZE - 4))) goto _output_error; LZ4_SECURECOPY(ref, op, cpy); op = cpy; /* correction */ @@ -279,7 +279,7 @@ static int lz4_uncompress_unknownoutputs goto _output_error; continue; } - if (unlikely((unsigned long)cpy < (unsigned long)op)) + if (unlikely((unsigned long)cpy < (unsigned long)op - (STEPSIZE - 4))) goto _output_error; LZ4_SECURECOPY(ref, op, cpy); op = cpy; /* correction */ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
There's no reason to allocate the dec{32,64}table on the stack; it
just wastes a bunch of instructions setting them up and, of course,
also consumes quite a bit of stack. Using size_t for such small
integers is a little excessive.
$ scripts/bloat-o-meter /tmp/built-in.o lib/built-in.o
add/remove: 2/2 grow/shrink: 2/0 up/down: 1304/-1548 (-244)
function old new delta
lz4_decompress_unknownoutputsize 55 718 +663
lz4_decompress 55 632 +577
dec64table - 32 +32
dec32table - 32 +32
lz4_uncompress 747 - -747
lz4_uncompress_unknownoutputsize 801 - -801
The now inlined lz4_uncompress functions used to have a stack
footprint of 176 bytes (according to -fstack-usage); their inlinees
have increased their stack use from 32 bytes to 48 and 80 bytes,
respectively.
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
[Linux commit bea2b592fd18eb8ffa3fc4ad380610632d03a38f]
Use {,u}int8_t instead of plain "int" for the tables.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/lz4/decompress.c
+++ b/xen/common/lz4/decompress.c
@@ -39,6 +39,11 @@
#include "defs.h"
+static const uint8_t dec32table[] = {0, 3, 2, 3, 0, 0, 0, 0};
+#if LZ4_ARCH64
+static const int8_t dec64table[] = {0, 0, 0, -1, 0, 1, 2, 3};
+#endif
+
#if defined(__XEN__) || defined(__MINIOS__)
static int INIT lz4_uncompress(const unsigned char *source, unsigned char *dest,
@@ -51,10 +56,6 @@ static int INIT lz4_uncompress(const uns
BYTE *cpy;
unsigned token;
size_t length;
- size_t dec32table[] = {0, 3, 2, 3, 0, 0, 0, 0};
-#if LZ4_ARCH64
- size_t dec64table[] = {0, 0, 0, -1, 0, 1, 2, 3};
-#endif
while (1) {
@@ -109,7 +110,7 @@ static int INIT lz4_uncompress(const uns
/* copy repeated sequence */
if (unlikely((op - ref) < STEPSIZE)) {
#if LZ4_ARCH64
- size_t dec64 = dec64table[op - ref];
+ int dec64 = dec64table[op - ref];
#else
const int dec64 = 0;
#endif
@@ -175,11 +176,6 @@ static int lz4_uncompress_unknownoutputs
BYTE * const oend = op + maxoutputsize;
BYTE *cpy;
- size_t dec32table[] = {0, 3, 2, 3, 0, 0, 0, 0};
-#if LZ4_ARCH64
- size_t dec64table[] = {0, 0, 0, -1, 0, 1, 2, 3};
-#endif
-
/* Main Loop */
while (ip < iend) {
@@ -245,7 +241,7 @@ static int lz4_uncompress_unknownoutputs
/* copy repeated sequence */
if (unlikely((op - ref) < STEPSIZE)) {
#if LZ4_ARCH64
- size_t dec64 = dec64table[op - ref];
+ int dec64 = dec64table[op - ref];
#else
const int dec64 = 0;
#endif
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
From: Krzysztof Kolasa <kkolasa@winsoft.pl>
Sometimes, on x86_64, decompression fails with the following
error:
Decompressing Linux...
Decoding failed
-- System halted
This condition is not needed for a 64bit kernel(from commit d5e7caf):
if( ... ||
(op + COPYLENGTH) > oend)
goto _output_error
macro LZ4_SECURE_COPY() tests op and does not copy any data
when op exceeds the value.
added by analogy to lz4_uncompress_unknownoutputsize(...)
Signed-off-by: Krzysztof Kolasa <kkolasa@winsoft.pl>
[Linux commit 99b7e93c95c78952724a9783de6c78def8fbfc3f]
The offending commit in our case is fcc17f96c277 ("LZ4 : fix the data
abort issue").
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/lz4/decompress.c
+++ b/xen/common/lz4/decompress.c
@@ -133,8 +133,12 @@ static int INIT lz4_uncompress(const uns
/* Error: request to write beyond destination buffer */
if (cpy > oend)
goto _output_error;
+#if LZ4_ARCH64
+ if ((ref + COPYLENGTH) > oend)
+#else
if ((ref + COPYLENGTH) > oend ||
(op + COPYLENGTH) > oend)
+#endif
goto _output_error;
LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH));
while (op < cpy)
@@ -262,7 +266,13 @@ static int lz4_uncompress_unknownoutputs
if (cpy > oend - COPYLENGTH) {
if (cpy > oend)
goto _output_error; /* write outside of buf */
-
+#if LZ4_ARCH64
+ if ((ref + COPYLENGTH) > oend)
+#else
+ if ((ref + COPYLENGTH) > oend ||
+ (op + COPYLENGTH) > oend)
+#endif
+ goto _output_error;
LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH));
while (op < cpy)
*op++ = *ref++;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
© 2016 - 2024 Red Hat, Inc.