diff mbox series

[1/3] ARM: compressed: Pass the actual output length to the decompressor

Message ID 20230412212126.3966502-2-j.neuschaefer@gmx.net (mailing list archive)
State New, archived
Headers show
Series ARM ZSTD boot compression | expand

Commit Message

J. Neuschäfer April 12, 2023, 9:21 p.m. UTC
ZSTD writes outside of the space that is necessary for the uncompressed
data, when it is told it has unlimited output length. To fix this, pass
the actual output length (the length of the uncompressed kernel) to the
decompressor.

The uncompressed length is already stored as a little endian 32-bit
constant before the input_data_end symbol.

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
 arch/arm/boot/compressed/decompress.c |  4 ++--
 arch/arm/boot/compressed/misc.c       | 12 ++++++++++--
 2 files changed, 12 insertions(+), 4 deletions(-)

--
2.39.2

Comments

Linus Walleij April 12, 2023, 9:42 p.m. UTC | #1
On Wed, Apr 12, 2023 at 11:21 PM Jonathan Neuschäfer
<j.neuschaefer@gmx.net> wrote:

> ZSTD writes outside of the space that is necessary for the uncompressed
> data, when it is told it has unlimited output length. To fix this, pass
> the actual output length (the length of the uncompressed kernel) to the
> decompressor.
>
> The uncompressed length is already stored as a little endian 32-bit
> constant before the input_data_end symbol.
>
> Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>

Really neat fix!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
Florian Fainelli April 12, 2023, 9:48 p.m. UTC | #2
On 4/12/23 14:21, Jonathan Neuschäfer wrote:
> ZSTD writes outside of the space that is necessary for the uncompressed
> data, when it is told it has unlimited output length. To fix this, pass
> the actual output length (the length of the uncompressed kernel) to the
> decompressor.
> 
> The uncompressed length is already stored as a little endian 32-bit
> constant before the input_data_end symbol.
> 
> Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Tony Lindgren April 13, 2023, 5:20 a.m. UTC | #3
* Jonathan Neuschäfer <j.neuschaefer@gmx.net> [230412 21:22]:
> --- a/arch/arm/boot/compressed/misc.c
> +++ b/arch/arm/boot/compressed/misc.c
> +static u32 get_inflated_image_size(void)
> +{
> +	return get_unaligned_le32(input_data_end - 4);
> +}

Just something to check.. This patch should not picked for the old stable
kernels that did not have the uncompressed image size at the end. Maybe
the patch should have a Depends-on tag to prevent possible issues?

Other than that looks good to me:

Reviewed-by: Tony Lindgren <tony@atomide.com>
J. Neuschäfer April 15, 2023, 1:52 a.m. UTC | #4
On Thu, Apr 13, 2023 at 08:20:50AM +0300, Tony Lindgren wrote:
> * Jonathan Neuschäfer <j.neuschaefer@gmx.net> [230412 21:22]:
> > --- a/arch/arm/boot/compressed/misc.c
> > +++ b/arch/arm/boot/compressed/misc.c
> > +static u32 get_inflated_image_size(void)
> > +{
> > +	return get_unaligned_le32(input_data_end - 4);
> > +}
> 
> Just something to check.. This patch should not picked for the old stable
> kernels that did not have the uncompressed image size at the end. Maybe
> the patch should have a Depends-on tag to prevent possible issues?

As far as I can see, the appended size has been around for a really long
time (v2.6.28, commit bc22c17e12c130dc929218a95aa347e0f3fd05dc), far
longer than the oldest LTS kernel that's still around.

> 
> Other than that looks good to me:
> 
> Reviewed-by: Tony Lindgren <tony@atomide.com>

Thanks

Jonathan
diff mbox series

Patch

diff --git a/arch/arm/boot/compressed/decompress.c b/arch/arm/boot/compressed/decompress.c
index 74255e8198314..3d098b84ee391 100644
--- a/arch/arm/boot/compressed/decompress.c
+++ b/arch/arm/boot/compressed/decompress.c
@@ -59,7 +59,7 @@  extern char * strchrnul(const char *, int);
 #include "../../../../lib/decompress_unlz4.c"
 #endif

-int do_decompress(u8 *input, int len, u8 *output, void (*error)(char *x))
+int do_decompress(u8 *input, int len, u8 *output, int outlen, void (*error)(char *x))
 {
-	return __decompress(input, len, NULL, NULL, output, 0, NULL, error);
+	return __decompress(input, len, NULL, NULL, output, outlen, NULL, error);
 }
diff --git a/arch/arm/boot/compressed/misc.c b/arch/arm/boot/compressed/misc.c
index abfed1aa2baa8..8402b29bccc82 100644
--- a/arch/arm/boot/compressed/misc.c
+++ b/arch/arm/boot/compressed/misc.c
@@ -22,6 +22,7 @@  unsigned int __machine_arch_type;
 #include <linux/compiler.h>	/* for inline */
 #include <linux/types.h>
 #include <linux/linkage.h>
+#include <asm/unaligned.h>
 #include "misc.h"
 #ifdef CONFIG_ARCH_EP93XX
 #include "misc-ep93xx.h"
@@ -131,17 +132,24 @@  asmlinkage void __div0(void)
 	error("Attempting division by 0!");
 }

-extern int do_decompress(u8 *input, int len, u8 *output, void (*error)(char *x));
+extern int do_decompress(u8 *input, int len, u8 *output, int outlen,
+			 void (*error)(char *x));

+static u32 get_inflated_image_size(void)
+{
+	return get_unaligned_le32(input_data_end - 4);
+}

 void
 decompress_kernel(unsigned long output_start, unsigned long free_mem_ptr_p,
 		unsigned long free_mem_ptr_end_p,
 		int arch_id)
 {
+	unsigned long output_data_len;
 	int ret;

 	output_data		= (unsigned char *)output_start;
+	output_data_len		= get_inflated_image_size();
 	free_mem_ptr		= free_mem_ptr_p;
 	free_mem_end_ptr	= free_mem_ptr_end_p;
 	__machine_arch_type	= arch_id;
@@ -153,7 +161,7 @@  decompress_kernel(unsigned long output_start, unsigned long free_mem_ptr_p,

 	putstr("Uncompressing Linux...");
 	ret = do_decompress(input_data, input_data_end - input_data,
-			    output_data, error);
+			    output_data, output_data_len, error);
 	if (ret)
 		error("decompressor returned an error");
 	else