* [PATCH v5 0/2] x86/boot: Warn on future overlapping memcpy() use @ 2016-05-02 22:50 Kees Cook 2016-05-02 22:51 ` [PATCH v5 1/2] x86/boot: Extract error reporting functions Kees Cook 2016-05-02 22:51 ` [PATCH v5 2/2] x86/boot: Warn on future overlapping memcpy() use Kees Cook 0 siblings, 2 replies; 5+ messages in thread From: Kees Cook @ 2016-05-02 22:50 UTC (permalink / raw) To: Ingo Molnar Cc: Kees Cook, Lasse Collin, One Thousand Gnomes, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, Yinghai Lu, Baoquan He, Borislav Petkov, x86@kernel.org, LKML This attempts to bring some sanity to how warn() and error() are defined so that they can be used by misc.c, kaslr.c, and string.c. After that, we add a warn() to memcpy and call memmove on detected overlaps. -Kees v5: - split out warn/error into error.c/error.h v4: - use __memcpy not memcpy since we've already done the check. v3: - call memmove in addition to doing the warning v2: - warn about overlapping region ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v5 1/2] x86/boot: Extract error reporting functions 2016-05-02 22:50 [PATCH v5 0/2] x86/boot: Warn on future overlapping memcpy() use Kees Cook @ 2016-05-02 22:51 ` Kees Cook 2016-05-03 7:45 ` [tip:x86/boot] " tip-bot for Kees Cook 2016-05-02 22:51 ` [PATCH v5 2/2] x86/boot: Warn on future overlapping memcpy() use Kees Cook 1 sibling, 1 reply; 5+ messages in thread From: Kees Cook @ 2016-05-02 22:51 UTC (permalink / raw) To: Ingo Molnar Cc: Kees Cook, Lasse Collin, One Thousand Gnomes, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, Yinghai Lu, Baoquan He, Borislav Petkov, x86@kernel.org, LKML Currently to use warn(), a caller would need to include misc.h. However, this means they would get the (unavailable during compressed boot) gcc built-in memcpy family of functions. But since string.c is defining these memcpy functions for use by misc.c, we end up in a weird circular dependency. To break this loop, move the error reporting functions outside of misc.c with their own header so that they can be independently included by other sources. Since the screen-writing routines use memmove(), keep the low-level *_putstr() functions in misc.c. Signed-off-by: Kees Cook <keescook@chromium.org> --- arch/x86/boot/compressed/Makefile | 2 +- arch/x86/boot/compressed/error.c | 22 ++++++++++++++++++++++ arch/x86/boot/compressed/error.h | 7 +++++++ arch/x86/boot/compressed/kaslr.c | 1 + arch/x86/boot/compressed/misc.c | 18 +----------------- arch/x86/boot/compressed/misc.h | 1 - arch/x86/boot/compressed/string.c | 2 ++ 7 files changed, 34 insertions(+), 19 deletions(-) create mode 100644 arch/x86/boot/compressed/error.c create mode 100644 arch/x86/boot/compressed/error.h diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile index 75f2233b8414..77ce3a04d46e 100644 --- a/arch/x86/boot/compressed/Makefile +++ b/arch/x86/boot/compressed/Makefile @@ -70,7 +70,7 @@ $(obj)/../voffset.h: vmlinux FORCE $(obj)/misc.o: $(obj)/../voffset.h vmlinux-objs-y := $(obj)/vmlinux.lds $(obj)/head_$(BITS).o $(obj)/misc.o \ - $(obj)/string.o $(obj)/cmdline.o \ + $(obj)/string.o $(obj)/cmdline.o $(obj)/error.o \ $(obj)/piggy.o $(obj)/cpuflags.o vmlinux-objs-$(CONFIG_EARLY_PRINTK) += $(obj)/early_serial_console.o diff --git a/arch/x86/boot/compressed/error.c b/arch/x86/boot/compressed/error.c new file mode 100644 index 000000000000..6248740b68b5 --- /dev/null +++ b/arch/x86/boot/compressed/error.c @@ -0,0 +1,22 @@ +/* + * Callers outside of misc.c need access to the error reporting routines, + * but the *_putstr() functions need to stay in misc.c because of how + * memcpy() and memmove() are defined for the compressed boot environment. + */ +#include "misc.h" + +void warn(char *m) +{ + error_putstr("\n\n"); + error_putstr(m); + error_putstr("\n\n"); +} + +void error(char *m) +{ + warn(m); + error_putstr(" -- System halted"); + + while (1) + asm("hlt"); +} diff --git a/arch/x86/boot/compressed/error.h b/arch/x86/boot/compressed/error.h new file mode 100644 index 000000000000..2e59dac07f9e --- /dev/null +++ b/arch/x86/boot/compressed/error.h @@ -0,0 +1,7 @@ +#ifndef BOOT_COMPRESSED_ERROR_H +#define BOOT_COMPRESSED_ERROR_H + +void warn(char *m); +void error(char *m); + +#endif /* BOOT_COMPRESSED_ERROR_H */ diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index 8741a6d83bfe..f1818d95d726 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -10,6 +10,7 @@ * */ #include "misc.h" +#include "error.h" #include <asm/msr.h> #include <asm/archrandom.h> diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c index 8f0253d8c7ff..9536d778149e 100644 --- a/arch/x86/boot/compressed/misc.c +++ b/arch/x86/boot/compressed/misc.c @@ -12,6 +12,7 @@ */ #include "misc.h" +#include "error.h" #include "../string.h" #include "../voffset.h" @@ -36,7 +37,6 @@ #define memmove memmove /* Functions used by the included decompressor code below. */ -static void error(char *m); void *memmove(void *dest, const void *src, size_t n); /* @@ -169,22 +169,6 @@ void __puthex(unsigned long value) } } -void warn(char *m) -{ - error_putstr("\n\n"); - error_putstr(m); - error_putstr("\n\n"); -} - -static void error(char *m) -{ - warn(m); - error_putstr(" -- System halted"); - - while (1) - asm("hlt"); -} - #if CONFIG_X86_NEED_RELOCS static void handle_relocations(void *output, unsigned long output_len) { diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h index e75f6cf9caaf..9887e0d4aaeb 100644 --- a/arch/x86/boot/compressed/misc.h +++ b/arch/x86/boot/compressed/misc.h @@ -35,7 +35,6 @@ extern memptr free_mem_end_ptr; extern struct boot_params *boot_params; void __putstr(const char *s); void __puthex(unsigned long value); -void warn(char *m); #define error_putstr(__x) __putstr(__x) #define error_puthex(__x) __puthex(__x) diff --git a/arch/x86/boot/compressed/string.c b/arch/x86/boot/compressed/string.c index 2befeca1aada..faa4dc7dc66b 100644 --- a/arch/x86/boot/compressed/string.c +++ b/arch/x86/boot/compressed/string.c @@ -5,6 +5,8 @@ * trust the gcc built-in implementations as they may do unexpected things * (e.g. FPU ops) in the minimal decompression stub execution environment. */ +#include "error.h" + #include "../string.c" #ifdef CONFIG_X86_32 -- 2.6.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [tip:x86/boot] x86/boot: Extract error reporting functions 2016-05-02 22:51 ` [PATCH v5 1/2] x86/boot: Extract error reporting functions Kees Cook @ 2016-05-03 7:45 ` tip-bot for Kees Cook 0 siblings, 0 replies; 5+ messages in thread From: tip-bot for Kees Cook @ 2016-05-03 7:45 UTC (permalink / raw) To: linux-tip-commits Cc: hpa, bp, dvlasenk, keescook, linux-kernel, tglx, mingo, lasse.collin, luto, peterz, yinghai, bp, bhe, torvalds, gnomes, brgerst Commit-ID: dc425a6e140bca99bdb4823e9909c9d9b8ba36b6 Gitweb: http://git.kernel.org/tip/dc425a6e140bca99bdb4823e9909c9d9b8ba36b6 Author: Kees Cook <keescook@chromium.org> AuthorDate: Mon, 2 May 2016 15:51:00 -0700 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Tue, 3 May 2016 08:15:58 +0200 x86/boot: Extract error reporting functions Currently to use warn(), a caller would need to include misc.h. However, this means they would get the (unavailable during compressed boot) gcc built-in memcpy family of functions. But since string.c is defining these memcpy functions for use by misc.c, we end up in a weird circular dependency. To break this loop, move the error reporting functions outside of misc.c with their own header so that they can be independently included by other sources. Since the screen-writing routines use memmove(), keep the low-level *_putstr() functions in misc.c. Signed-off-by: Kees Cook <keescook@chromium.org> Cc: Andy Lutomirski <luto@amacapital.net> Cc: Baoquan He <bhe@redhat.com> Cc: Borislav Petkov <bp@alien8.de> Cc: Borislav Petkov <bp@suse.de> Cc: Brian Gerst <brgerst@gmail.com> Cc: Denys Vlasenko <dvlasenk@redhat.com> Cc: H. Peter Anvin <hpa@zytor.com> Cc: Lasse Collin <lasse.collin@tukaani.org> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Yinghai Lu <yinghai@kernel.org> Link: http://lkml.kernel.org/r/1462229461-3370-2-git-send-email-keescook@chromium.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/boot/compressed/Makefile | 2 +- arch/x86/boot/compressed/error.c | 22 ++++++++++++++++++++++ arch/x86/boot/compressed/error.h | 7 +++++++ arch/x86/boot/compressed/kaslr.c | 1 + arch/x86/boot/compressed/misc.c | 18 +----------------- arch/x86/boot/compressed/misc.h | 1 - arch/x86/boot/compressed/string.c | 2 ++ 7 files changed, 34 insertions(+), 19 deletions(-) diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile index 75f2233..77ce3a0 100644 --- a/arch/x86/boot/compressed/Makefile +++ b/arch/x86/boot/compressed/Makefile @@ -70,7 +70,7 @@ $(obj)/../voffset.h: vmlinux FORCE $(obj)/misc.o: $(obj)/../voffset.h vmlinux-objs-y := $(obj)/vmlinux.lds $(obj)/head_$(BITS).o $(obj)/misc.o \ - $(obj)/string.o $(obj)/cmdline.o \ + $(obj)/string.o $(obj)/cmdline.o $(obj)/error.o \ $(obj)/piggy.o $(obj)/cpuflags.o vmlinux-objs-$(CONFIG_EARLY_PRINTK) += $(obj)/early_serial_console.o diff --git a/arch/x86/boot/compressed/error.c b/arch/x86/boot/compressed/error.c new file mode 100644 index 0000000..6248740 --- /dev/null +++ b/arch/x86/boot/compressed/error.c @@ -0,0 +1,22 @@ +/* + * Callers outside of misc.c need access to the error reporting routines, + * but the *_putstr() functions need to stay in misc.c because of how + * memcpy() and memmove() are defined for the compressed boot environment. + */ +#include "misc.h" + +void warn(char *m) +{ + error_putstr("\n\n"); + error_putstr(m); + error_putstr("\n\n"); +} + +void error(char *m) +{ + warn(m); + error_putstr(" -- System halted"); + + while (1) + asm("hlt"); +} diff --git a/arch/x86/boot/compressed/error.h b/arch/x86/boot/compressed/error.h new file mode 100644 index 0000000..2e59dac --- /dev/null +++ b/arch/x86/boot/compressed/error.h @@ -0,0 +1,7 @@ +#ifndef BOOT_COMPRESSED_ERROR_H +#define BOOT_COMPRESSED_ERROR_H + +void warn(char *m); +void error(char *m); + +#endif /* BOOT_COMPRESSED_ERROR_H */ diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index 8741a6d..f1818d9 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -10,6 +10,7 @@ * */ #include "misc.h" +#include "error.h" #include <asm/msr.h> #include <asm/archrandom.h> diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c index 8f0253d..9536d77 100644 --- a/arch/x86/boot/compressed/misc.c +++ b/arch/x86/boot/compressed/misc.c @@ -12,6 +12,7 @@ */ #include "misc.h" +#include "error.h" #include "../string.h" #include "../voffset.h" @@ -36,7 +37,6 @@ #define memmove memmove /* Functions used by the included decompressor code below. */ -static void error(char *m); void *memmove(void *dest, const void *src, size_t n); /* @@ -169,22 +169,6 @@ void __puthex(unsigned long value) } } -void warn(char *m) -{ - error_putstr("\n\n"); - error_putstr(m); - error_putstr("\n\n"); -} - -static void error(char *m) -{ - warn(m); - error_putstr(" -- System halted"); - - while (1) - asm("hlt"); -} - #if CONFIG_X86_NEED_RELOCS static void handle_relocations(void *output, unsigned long output_len) { diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h index e75f6cf..9887e0d 100644 --- a/arch/x86/boot/compressed/misc.h +++ b/arch/x86/boot/compressed/misc.h @@ -35,7 +35,6 @@ extern memptr free_mem_end_ptr; extern struct boot_params *boot_params; void __putstr(const char *s); void __puthex(unsigned long value); -void warn(char *m); #define error_putstr(__x) __putstr(__x) #define error_puthex(__x) __puthex(__x) diff --git a/arch/x86/boot/compressed/string.c b/arch/x86/boot/compressed/string.c index 2befeca..faa4dc7 100644 --- a/arch/x86/boot/compressed/string.c +++ b/arch/x86/boot/compressed/string.c @@ -5,6 +5,8 @@ * trust the gcc built-in implementations as they may do unexpected things * (e.g. FPU ops) in the minimal decompression stub execution environment. */ +#include "error.h" + #include "../string.c" #ifdef CONFIG_X86_32 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v5 2/2] x86/boot: Warn on future overlapping memcpy() use 2016-05-02 22:50 [PATCH v5 0/2] x86/boot: Warn on future overlapping memcpy() use Kees Cook 2016-05-02 22:51 ` [PATCH v5 1/2] x86/boot: Extract error reporting functions Kees Cook @ 2016-05-02 22:51 ` Kees Cook 2016-05-03 7:46 ` [tip:x86/boot] " tip-bot for Kees Cook 1 sibling, 1 reply; 5+ messages in thread From: Kees Cook @ 2016-05-02 22:51 UTC (permalink / raw) To: Ingo Molnar Cc: Kees Cook, Lasse Collin, One Thousand Gnomes, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, Yinghai Lu, Baoquan He, Borislav Petkov, x86@kernel.org, LKML If an overlapping memcpy() is ever attempted, we should at least report it, in case it might lead to problems, so it could be changed to a memmove() call instead. Suggested-by: Ingo Molnar <mingo@kernel.org> Signed-off-by: Kees Cook <keescook@chromium.org> --- arch/x86/boot/compressed/string.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/arch/x86/boot/compressed/string.c b/arch/x86/boot/compressed/string.c index faa4dc7dc66b..cea140ce6b42 100644 --- a/arch/x86/boot/compressed/string.c +++ b/arch/x86/boot/compressed/string.c @@ -10,7 +10,7 @@ #include "../string.c" #ifdef CONFIG_X86_32 -void *memcpy(void *dest, const void *src, size_t n) +static void *__memcpy(void *dest, const void *src, size_t n) { int d0, d1, d2; asm volatile( @@ -24,7 +24,7 @@ void *memcpy(void *dest, const void *src, size_t n) return dest; } #else -void *memcpy(void *dest, const void *src, size_t n) +static void *__memcpy(void *dest, const void *src, size_t n) { long d0, d1, d2; asm volatile( @@ -55,10 +55,20 @@ void *memmove(void *dest, const void *src, size_t n) const unsigned char *s = src; if (d <= s || d - s >= n) - return memcpy(dest, src, n); + return __memcpy(dest, src, n); while (n-- > 0) d[n] = s[n]; return dest; } + +/* Detect and warn about potential overlaps, but handle them with memmove. */ +void *memcpy(void *dest, const void *src, size_t n) +{ + if (dest > src && dest - src < n) { + warn("Avoiding potentially unsafe overlapping memcpy()!"); + return memmove(dest, src, n); + } + return __memcpy(dest, src, n); +} -- 2.6.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [tip:x86/boot] x86/boot: Warn on future overlapping memcpy() use 2016-05-02 22:51 ` [PATCH v5 2/2] x86/boot: Warn on future overlapping memcpy() use Kees Cook @ 2016-05-03 7:46 ` tip-bot for Kees Cook 0 siblings, 0 replies; 5+ messages in thread From: tip-bot for Kees Cook @ 2016-05-03 7:46 UTC (permalink / raw) To: linux-tip-commits Cc: mingo, brgerst, tglx, yinghai, gnomes, dvlasenk, bp, bp, keescook, bhe, hpa, linux-kernel, peterz, luto, lasse.collin, torvalds Commit-ID: 00ec2c37031eb1b1feda006c84748d126dc2ef27 Gitweb: http://git.kernel.org/tip/00ec2c37031eb1b1feda006c84748d126dc2ef27 Author: Kees Cook <keescook@chromium.org> AuthorDate: Mon, 2 May 2016 15:51:01 -0700 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Tue, 3 May 2016 08:15:58 +0200 x86/boot: Warn on future overlapping memcpy() use If an overlapping memcpy() is ever attempted, we should at least report it, in case it might lead to problems, so it could be changed to a memmove() call instead. Suggested-by: Ingo Molnar <mingo@kernel.org> Signed-off-by: Kees Cook <keescook@chromium.org> Cc: Andy Lutomirski <luto@amacapital.net> Cc: Baoquan He <bhe@redhat.com> Cc: Borislav Petkov <bp@alien8.de> Cc: Borislav Petkov <bp@suse.de> Cc: Brian Gerst <brgerst@gmail.com> Cc: Denys Vlasenko <dvlasenk@redhat.com> Cc: H. Peter Anvin <hpa@zytor.com> Cc: Lasse Collin <lasse.collin@tukaani.org> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Yinghai Lu <yinghai@kernel.org> Link: http://lkml.kernel.org/r/1462229461-3370-3-git-send-email-keescook@chromium.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/boot/compressed/string.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/arch/x86/boot/compressed/string.c b/arch/x86/boot/compressed/string.c index faa4dc7..cea140c 100644 --- a/arch/x86/boot/compressed/string.c +++ b/arch/x86/boot/compressed/string.c @@ -10,7 +10,7 @@ #include "../string.c" #ifdef CONFIG_X86_32 -void *memcpy(void *dest, const void *src, size_t n) +static void *__memcpy(void *dest, const void *src, size_t n) { int d0, d1, d2; asm volatile( @@ -24,7 +24,7 @@ void *memcpy(void *dest, const void *src, size_t n) return dest; } #else -void *memcpy(void *dest, const void *src, size_t n) +static void *__memcpy(void *dest, const void *src, size_t n) { long d0, d1, d2; asm volatile( @@ -55,10 +55,20 @@ void *memmove(void *dest, const void *src, size_t n) const unsigned char *s = src; if (d <= s || d - s >= n) - return memcpy(dest, src, n); + return __memcpy(dest, src, n); while (n-- > 0) d[n] = s[n]; return dest; } + +/* Detect and warn about potential overlaps, but handle them with memmove. */ +void *memcpy(void *dest, const void *src, size_t n) +{ + if (dest > src && dest - src < n) { + warn("Avoiding potentially unsafe overlapping memcpy()!"); + return memmove(dest, src, n); + } + return __memcpy(dest, src, n); +} ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-05-03 7:48 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-05-02 22:50 [PATCH v5 0/2] x86/boot: Warn on future overlapping memcpy() use Kees Cook 2016-05-02 22:51 ` [PATCH v5 1/2] x86/boot: Extract error reporting functions Kees Cook 2016-05-03 7:45 ` [tip:x86/boot] " tip-bot for Kees Cook 2016-05-02 22:51 ` [PATCH v5 2/2] x86/boot: Warn on future overlapping memcpy() use Kees Cook 2016-05-03 7:46 ` [tip:x86/boot] " tip-bot for Kees Cook
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.