* [PATCH v2 1/2] x86/purgatory: do not use __builtin_memcpy and __builtin_memset
@ 2019-07-22 21:32 Nick Desaulniers
2019-07-22 21:32 ` [PATCH v2 2/2] x86/purgatory: use CFLAGS_REMOVE rather than reset KBUILD_CFLAGS Nick Desaulniers
[not found] ` <20190722213250.238685-3-ndesaulniers@google.com>
0 siblings, 2 replies; 6+ messages in thread
From: Nick Desaulniers @ 2019-07-22 21:32 UTC (permalink / raw)
To: tglx, mingo, bp
Cc: peterz, clang-built-linux, Nick Desaulniers, Vaibhav Rustagi,
Manoj Gupta, Alistair Delva, H. Peter Anvin, x86, Allison Randal,
Alexios Zavras, Enrico Weigelt, Greg Kroah-Hartman, linux-kernel
Implementing memcpy and memset in terms of __builtin_memcpy and
__builtin_memset is problematic.
GCC at -O2 will replace calls to the builtins with calls to memcpy and
memset (but will generate an inline implementation at -Os). Clang will
replace the builtins with these calls regardless of optimization level.
$ llvm-objdump -dr arch/x86/purgatory/string.o | tail
0000000000000339 memcpy:
339: 48 b8 00 00 00 00 00 00 00 00 movabsq $0, %rax
000000000000033b: R_X86_64_64 memcpy
343: ff e0 jmpq *%rax
0000000000000345 memset:
345: 48 b8 00 00 00 00 00 00 00 00 movabsq $0, %rax
0000000000000347: R_X86_64_64 memset
34f: ff e0
Such code results in infinite recursion at runtime. This is observed
when doing kexec.
Instead, reuse an implementation from arch/x86/boot/compressed/string.c
if we define warn as a symbol.
Fixes: 8fc5b4d4121c ("purgatory: core purgatory functionality")
Link: https://bugs.chromium.org/p/chromium/issues/detail?id=984056
Reported-by: Vaibhav Rustagi <vaibhavrustagi@google.com>
Tested-by: Vaibhav Rustagi <vaibhavrustagi@google.com>
Debugged-by: Vaibhav Rustagi <vaibhavrustagi@google.com>
Debugged-by: Manoj Gupta <manojgupta@google.com>
Suggested-by: Alistair Delva <adelva@google.com>
Signed-off-by: Vaibhav Rustagi <vaibhavrustagi@google.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Changes v1 -> v2:
* Add Fixes tag.
* Move this patch to first in the series.
arch/x86/purgatory/Makefile | 3 +++
arch/x86/purgatory/purgatory.c | 6 ++++++
arch/x86/purgatory/string.c | 23 -----------------------
3 files changed, 9 insertions(+), 23 deletions(-)
delete mode 100644 arch/x86/purgatory/string.c
diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
index 3cf302b26332..91ef244026d2 100644
--- a/arch/x86/purgatory/Makefile
+++ b/arch/x86/purgatory/Makefile
@@ -6,6 +6,9 @@ purgatory-y := purgatory.o stack.o setup-x86_$(BITS).o sha256.o entry64.o string
targets += $(purgatory-y)
PURGATORY_OBJS = $(addprefix $(obj)/,$(purgatory-y))
+$(obj)/string.o: $(srctree)/arch/x86/boot/compressed/string.c FORCE
+ $(call if_changed_rule,cc_o_c)
+
$(obj)/sha256.o: $(srctree)/lib/sha256.c FORCE
$(call if_changed_rule,cc_o_c)
diff --git a/arch/x86/purgatory/purgatory.c b/arch/x86/purgatory/purgatory.c
index 6d8d5a34c377..b607bda786f6 100644
--- a/arch/x86/purgatory/purgatory.c
+++ b/arch/x86/purgatory/purgatory.c
@@ -68,3 +68,9 @@ void purgatory(void)
}
copy_backup_region();
}
+
+/*
+ * Defined in order to reuse memcpy() and memset() from
+ * arch/x86/boot/compressed/string.c
+ */
+void warn(const char *msg) {}
diff --git a/arch/x86/purgatory/string.c b/arch/x86/purgatory/string.c
deleted file mode 100644
index 01ad43873ad9..000000000000
--- a/arch/x86/purgatory/string.c
+++ /dev/null
@@ -1,23 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * Simple string functions.
- *
- * Copyright (C) 2014 Red Hat Inc.
- *
- * Author:
- * Vivek Goyal <vgoyal@redhat.com>
- */
-
-#include <linux/types.h>
-
-#include "../boot/string.c"
-
-void *memcpy(void *dst, const void *src, size_t len)
-{
- return __builtin_memcpy(dst, src, len);
-}
-
-void *memset(void *dst, int c, size_t len)
-{
- return __builtin_memset(dst, c, len);
-}
--
2.22.0.657.g960e92d24f-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] x86/purgatory: use CFLAGS_REMOVE rather than reset KBUILD_CFLAGS
2019-07-22 21:32 [PATCH v2 1/2] x86/purgatory: do not use __builtin_memcpy and __builtin_memset Nick Desaulniers
@ 2019-07-22 21:32 ` Nick Desaulniers
2019-07-22 22:10 ` Nick Desaulniers
[not found] ` <20190722213250.238685-3-ndesaulniers@google.com>
1 sibling, 1 reply; 6+ messages in thread
From: Nick Desaulniers @ 2019-07-22 21:32 UTC (permalink / raw)
To: tglx, mingo, bp
Cc: peterz, clang-built-linux, Nick Desaulniers, Vaibhav Rustagi,
H. Peter Anvin, x86, linux-kernel
KBUILD_CFLAGS is very carefully built up in the top level Makefile,
particularly when cross compiling or using different build tools.
Resetting KBUILD_CFLAGS via := assignment is an antipattern.
The comment above the reset mentions that -pg is problematic. Other
Makefiles like arch/x86/xen/vdso/Makefile use
`CFLAGS_REMOVE_file.o = -pg` when CONFIG_FUNCTION_TRACER is set. Prefer
that pattern to wiping out all of the important KBUILD_CFLAGS then
manually having to re-add them.
Fixes: 8fc5b4d4121c ("purgatory: core purgatory functionality")
Reported-by: Vaibhav Rustagi <vaibhavrustagi@google.com>
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Rather than manually add -mno-sse, -mno-mmx, -mno-sse2, prefer to filter
-pg flags.
arch/x86/purgatory/Makefile | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
index 91ef244026d2..56bcabca283f 100644
--- a/arch/x86/purgatory/Makefile
+++ b/arch/x86/purgatory/Makefile
@@ -20,11 +20,13 @@ KCOV_INSTRUMENT := n
# Default KBUILD_CFLAGS can have -pg option set when FTRACE is enabled. That
# in turn leaves some undefined symbols like __fentry__ in purgatory and not
-# sure how to relocate those. Like kexec-tools, use custom flags.
-
-KBUILD_CFLAGS := -fno-strict-aliasing -Wall -Wstrict-prototypes -fno-zero-initialized-in-bss -fno-builtin -ffreestanding -c -Os -mcmodel=large
-KBUILD_CFLAGS += -m$(BITS)
-KBUILD_CFLAGS += $(call cc-option,-fno-PIE)
+# sure how to relocate those.
+ifdef CONFIG_FUNCTION_TRACER
+CFLAGS_REMOVE_sha256.o = -pg
+CFLAGS_REMOVE_purgatory.o = -pg
+CFLAGS_REMOVE_string.o = -pg
+CFLAGS_REMOVE_kexec-purgatory.o = -pg
+endif
$(obj)/purgatory.ro: $(PURGATORY_OBJS) FORCE
$(call if_changed,ld)
--
2.22.0.657.g960e92d24f-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/2] Support kexec/kdump for clang built kernel
[not found] ` <20190722213250.238685-3-ndesaulniers@google.com>
@ 2019-07-22 21:41 ` Nick Desaulniers
0 siblings, 0 replies; 6+ messages in thread
From: Nick Desaulniers @ 2019-07-22 21:41 UTC (permalink / raw)
To: Joe Perches
Cc: Peter Zijlstra, clang-built-linux, Ingo Molnar, Borislav Petkov,
Thomas Gleixner, LKML
Joe,
Is it possible to have scripts/get_maintainer.pl always cc
linux-kernel@vger.kernel.org? I just sent out a series, and it seems
the cover letter didn't get sent to LKML. I usually use this shell
function to send patches:
```
function kpatch () {
patch=$1
shift
git send-email \
--cc-cmd="./scripts/get_maintainer.pl --norolestats $patch" \
$@ $patch
}
```
Invoked via:
```
$ mkdir purgatory
$ git format-patch HEAD~2 --cover-letter -o purgatory -v2
$ kpatch purgatory/v2-000* --cc peterz@infradead.org --cc
clang-built-linux@googlegroups.com
```
Maybe I should just add `--cc linux-kernel@vger.kernel.org` to my
shell function?
On Mon, Jul 22, 2019 at 2:33 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> 1. Reuse the implementation of memcpy and memset instead of relying on
> __builtin_memcpy and __builtin_memset as it causes infinite recursion
> in Clang (at any opt level) or GCC at -O2.
> 2. Don't reset KBUILD_CFLAGS, rather filter CONFIG_FUNCTION_TRACER flags
> via `CFLAGS_REMOVE_<file>.o = -pg`.
>
> The order of the patches are reversed; in case we ever need to bisect,
> the memcpy/memset infinite recursion would occur with just patch 2/2
> applied.
>
> V2 of: https://lkml.org/lkml/2019/7/17/755
>
> Nick Desaulniers (2):
> x86/purgatory: do not use __builtin_memcpy and __builtin_memset
> x86/purgatory: use CFLAGS_REMOVE rather than reset KBUILD_CFLAGS
>
> arch/x86/purgatory/Makefile | 15 ++++++++++-----
> arch/x86/purgatory/purgatory.c | 6 ++++++
> arch/x86/purgatory/string.c | 23 -----------------------
> 3 files changed, 16 insertions(+), 28 deletions(-)
> delete mode 100644 arch/x86/purgatory/string.c
>
> --
> 2.22.0.657.g960e92d24f-goog
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] x86/purgatory: use CFLAGS_REMOVE rather than reset KBUILD_CFLAGS
2019-07-22 21:32 ` [PATCH v2 2/2] x86/purgatory: use CFLAGS_REMOVE rather than reset KBUILD_CFLAGS Nick Desaulniers
@ 2019-07-22 22:10 ` Nick Desaulniers
2019-07-22 22:59 ` Vaibhav Rustagi
0 siblings, 1 reply; 6+ messages in thread
From: Nick Desaulniers @ 2019-07-22 22:10 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov
Cc: Peter Zijlstra, clang-built-linux, Vaibhav Rustagi,
H. Peter Anvin, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
LKML
On Mon, Jul 22, 2019 at 2:33 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> KBUILD_CFLAGS is very carefully built up in the top level Makefile,
> particularly when cross compiling or using different build tools.
> Resetting KBUILD_CFLAGS via := assignment is an antipattern.
>
> The comment above the reset mentions that -pg is problematic. Other
> Makefiles like arch/x86/xen/vdso/Makefile use
> `CFLAGS_REMOVE_file.o = -pg` when CONFIG_FUNCTION_TRACER is set. Prefer
> that pattern to wiping out all of the important KBUILD_CFLAGS then
> manually having to re-add them.
>
> Fixes: 8fc5b4d4121c ("purgatory: core purgatory functionality")
> Reported-by: Vaibhav Rustagi <vaibhavrustagi@google.com>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> Rather than manually add -mno-sse, -mno-mmx, -mno-sse2, prefer to filter
> -pg flags.
>
> arch/x86/purgatory/Makefile | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
> index 91ef244026d2..56bcabca283f 100644
> --- a/arch/x86/purgatory/Makefile
> +++ b/arch/x86/purgatory/Makefile
> @@ -20,11 +20,13 @@ KCOV_INSTRUMENT := n
>
> # Default KBUILD_CFLAGS can have -pg option set when FTRACE is enabled. That
> # in turn leaves some undefined symbols like __fentry__ in purgatory and not
> -# sure how to relocate those. Like kexec-tools, use custom flags.
> -
> -KBUILD_CFLAGS := -fno-strict-aliasing -Wall -Wstrict-prototypes -fno-zero-initialized-in-bss -fno-builtin -ffreestanding -c -Os -mcmodel=large
> -KBUILD_CFLAGS += -m$(BITS)
Is purgatory/kexec supported for CONFIG_X86_32? Should I be keeping
`-m$(BITS)`? arch/x86/purgatory/Makefile mentions
`setup-x86_$(BITS).o` which I assume is broken as there is no
arch/x86/purgatory/setup-x86_32.S?
> -KBUILD_CFLAGS += $(call cc-option,-fno-PIE)
> +# sure how to relocate those.
> +ifdef CONFIG_FUNCTION_TRACER
> +CFLAGS_REMOVE_sha256.o = -pg
> +CFLAGS_REMOVE_purgatory.o = -pg
> +CFLAGS_REMOVE_string.o = -pg
> +CFLAGS_REMOVE_kexec-purgatory.o = -pg
> +endif
>
> $(obj)/purgatory.ro: $(PURGATORY_OBJS) FORCE
> $(call if_changed,ld)
> --
> 2.22.0.657.g960e92d24f-goog
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] x86/purgatory: use CFLAGS_REMOVE rather than reset KBUILD_CFLAGS
2019-07-22 22:10 ` Nick Desaulniers
@ 2019-07-22 22:59 ` Vaibhav Rustagi
2019-07-23 21:28 ` Nick Desaulniers
0 siblings, 1 reply; 6+ messages in thread
From: Vaibhav Rustagi @ 2019-07-22 22:59 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Peter Zijlstra,
clang-built-linux, H. Peter Anvin,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
LKML
On Mon, Jul 22, 2019 at 3:10 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Mon, Jul 22, 2019 at 2:33 PM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > KBUILD_CFLAGS is very carefully built up in the top level Makefile,
> > particularly when cross compiling or using different build tools.
> > Resetting KBUILD_CFLAGS via := assignment is an antipattern.
> >
> > The comment above the reset mentions that -pg is problematic. Other
> > Makefiles like arch/x86/xen/vdso/Makefile use
> > `CFLAGS_REMOVE_file.o = -pg` when CONFIG_FUNCTION_TRACER is set. Prefer
> > that pattern to wiping out all of the important KBUILD_CFLAGS then
> > manually having to re-add them.
> >
> > Fixes: 8fc5b4d4121c ("purgatory: core purgatory functionality")
> > Reported-by: Vaibhav Rustagi <vaibhavrustagi@google.com>
> > Suggested-by: Peter Zijlstra <peterz@infradead.org>
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > ---
> > Rather than manually add -mno-sse, -mno-mmx, -mno-sse2, prefer to filter
> > -pg flags.
> >
> > arch/x86/purgatory/Makefile | 12 +++++++-----
> > 1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
> > index 91ef244026d2..56bcabca283f 100644
> > --- a/arch/x86/purgatory/Makefile
> > +++ b/arch/x86/purgatory/Makefile
> > @@ -20,11 +20,13 @@ KCOV_INSTRUMENT := n
> >
> > # Default KBUILD_CFLAGS can have -pg option set when FTRACE is enabled. That
> > # in turn leaves some undefined symbols like __fentry__ in purgatory and not
> > -# sure how to relocate those. Like kexec-tools, use custom flags.
> > -
> > -KBUILD_CFLAGS := -fno-strict-aliasing -Wall -Wstrict-prototypes -fno-zero-initialized-in-bss -fno-builtin -ffreestanding -c -Os -mcmodel=large
> > -KBUILD_CFLAGS += -m$(BITS)
>
> Is purgatory/kexec supported for CONFIG_X86_32? Should I be keeping
> `-m$(BITS)`? arch/x86/purgatory/Makefile mentions
> `setup-x86_$(BITS).o` which I assume is broken as there is no
> arch/x86/purgatory/setup-x86_32.S?
>
> > -KBUILD_CFLAGS += $(call cc-option,-fno-PIE)
> > +# sure how to relocate those.
> > +ifdef CONFIG_FUNCTION_TRACER
> > +CFLAGS_REMOVE_sha256.o = -pg
> > +CFLAGS_REMOVE_purgatory.o = -pg
> > +CFLAGS_REMOVE_string.o = -pg
> > +CFLAGS_REMOVE_kexec-purgatory.o = -pg
> > +endif
> >
The changes suggested will cause undefined symbols while loading the new kernel.
On doing 'nm purgatory.ro', I found below undefined symbols:
U bmcp
U __stack_chk_fail
> > $(obj)/purgatory.ro: $(PURGATORY_OBJS) FORCE
> > $(call if_changed,ld)
> > --
> > 2.22.0.657.g960e92d24f-goog
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] x86/purgatory: use CFLAGS_REMOVE rather than reset KBUILD_CFLAGS
2019-07-22 22:59 ` Vaibhav Rustagi
@ 2019-07-23 21:28 ` Nick Desaulniers
0 siblings, 0 replies; 6+ messages in thread
From: Nick Desaulniers @ 2019-07-23 21:28 UTC (permalink / raw)
To: Vaibhav Rustagi
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Peter Zijlstra,
clang-built-linux, H. Peter Anvin,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
LKML
On Mon, Jul 22, 2019 at 3:59 PM Vaibhav Rustagi
<vaibhavrustagi@google.com> wrote:
> The changes suggested will cause undefined symbols while loading the new kernel.
> On doing 'nm purgatory.ro', I found below undefined symbols:
>
> U bmcp
> U __stack_chk_fail
Thanks for the report, a v3: https://lkml.org/lkml/2019/7/23/864
(Tested v3 with defconfig, defconfig+CONFIG_FUNCTION_TRACER,
+CONFIG_STACKPROTECTOR_STRONG, and allyesconfig)
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-07-23 21:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-22 21:32 [PATCH v2 1/2] x86/purgatory: do not use __builtin_memcpy and __builtin_memset Nick Desaulniers
2019-07-22 21:32 ` [PATCH v2 2/2] x86/purgatory: use CFLAGS_REMOVE rather than reset KBUILD_CFLAGS Nick Desaulniers
2019-07-22 22:10 ` Nick Desaulniers
2019-07-22 22:59 ` Vaibhav Rustagi
2019-07-23 21:28 ` Nick Desaulniers
[not found] ` <20190722213250.238685-3-ndesaulniers@google.com>
2019-07-22 21:41 ` [PATCH v2 0/2] Support kexec/kdump for clang built kernel Nick Desaulniers
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).