* Kees Cook <keesc...@chromium.org> wrote:
> Two uses of memcpy (screen scrolling and ELF parsing) were handling
> overlapping memory areas. While there were no explicitly noticed bugs
> here (yet), it is best to fix this so that the copying will always be
> safe.
>
> Instead of making a new memmove function that might collide with other
> memmove definitions in the decompressors, this just makes the compressed
> boot's copy of memcpy overlap safe.
>
> Reported-by: Yinghai Lu <ying...@kernel.org>
> Suggested-by: Lasse Collin <lasse.col...@tukaani.org>
> Signed-off-by: Kees Cook <keesc...@chromium.org>
> ---
> arch/x86/boot/compressed/misc.c | 4 +---
> arch/x86/boot/compressed/string.c | 22 ++++++++++++++++++++--
> 2 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/string.c
> b/arch/x86/boot/compressed/string.c
> index 00e788be1db9..1e10e40f49dd 100644
> --- a/arch/x86/boot/compressed/string.c
> +++ b/arch/x86/boot/compressed/string.c
> @@ -1,7 +1,7 @@
> #include "../string.c"
>
> #ifdef CONFIG_X86_32
I've applied this patch, but could you please also do another patch that adds a
comment block to the top of this special version of compressed/string.c, which
explains why this file exists and what its purpose is?
Also:
+/*
+ * This memcpy is overlap safe (i.e. it is memmove without conflicting
+ * with other definitions of memmove from the various decompressors.
+ */
+void *memcpy(void *dest, const void *src, size_t n)
I'd not name it memcpy() if its semantics are not the same as the regular
kernel
memcpy() - that will only cause confusion later on.
I'd try to name it memmove() and would fix the memmove() hacks in decompressors:
lib/decompress_unxz.c:#ifndef memmove
lib/decompress_unxz.c:void *memmove(void *dest, const void *src, size_t size)
lib/decompress_unxz.c: * Since we need memmove anyway, would use it as memcpy
too.
lib/decompress_unxz.c:# define memcpy memmove
any strong reason this cannot be done?
Some other decompressors seem to avoid memmove() intentionally:
lib/decompress_bunzip2.c: *by 256 in any case, using memmove
here would
lib/decompress_unlzo.c: * of the buffer. This way memmove() isn't
needed which
lib/decompress_unlzo.c: * Use a loop to avoid memmove()
dependency.
Thanks,
Ingo
