| Message ID | 20200817220212.338670-2-ndesaulniers@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | -ffreestanding/-fno-builtin-* patches | expand |
On 2020-08-17 15:02, Nick Desaulniers wrote: > LLVM implemented a recent "libcall optimization" that lowers calls to > `sprintf(dest, "%s", str)` where the return value is used to > `stpcpy(dest, str) - dest`. This generally avoids the machinery involved > in parsing format strings. This optimization was introduced into > clang-12. Because the kernel does not provide an implementation of > stpcpy, we observe linkage failures for almost all targets when building > with ToT clang. > > The interface is unsafe as it does not perform any bounds checking. > Disable this "libcall optimization" via `-fno-builtin-stpcpy`. > > Unlike > commit 5f074f3e192f ("lib/string.c: implement a basic bcmp") > which cited failures with `-fno-builtin-*` flags being retained in LLVM > LTO, that bug seems to have been fixed by > https://reviews.llvm.org/D71193, so the above sha can now be reverted in > favor of `-fno-builtin-bcmp`. > stpcpy() and (to a lesser degree) mempcpy() are fairly useful routines in general. Perhaps we *should* provide them? -hpa
On Mon, Aug 17, 2020 at 3:31 PM H. Peter Anvin <hpa@zytor.com> wrote: > > On 2020-08-17 15:02, Nick Desaulniers wrote: > > LLVM implemented a recent "libcall optimization" that lowers calls to > > `sprintf(dest, "%s", str)` where the return value is used to > > `stpcpy(dest, str) - dest`. This generally avoids the machinery involved > > in parsing format strings. This optimization was introduced into > > clang-12. Because the kernel does not provide an implementation of > > stpcpy, we observe linkage failures for almost all targets when building > > with ToT clang. > > > > The interface is unsafe as it does not perform any bounds checking. > > Disable this "libcall optimization" via `-fno-builtin-stpcpy`. > > > > Unlike > > commit 5f074f3e192f ("lib/string.c: implement a basic bcmp") > > which cited failures with `-fno-builtin-*` flags being retained in LLVM > > LTO, that bug seems to have been fixed by > > https://reviews.llvm.org/D71193, so the above sha can now be reverted in > > favor of `-fno-builtin-bcmp`. > > > > stpcpy() and (to a lesser degree) mempcpy() are fairly useful routines > in general. Perhaps we *should* provide them? Sorry, I forgot to provide context of the previous thread, which is worth a read. To answer this question specifically (or at least for stpcpy), the answer from the previous thread was (via Kees): "No; please no more unbounded string.h routines": https://lore.kernel.org/lkml/202008150921.B70721A359@keescook/
On Tue, 18 Aug 2020 at 00:02, Nick Desaulniers <ndesaulniers@google.com> wrote: > > LLVM implemented a recent "libcall optimization" that lowers calls to > `sprintf(dest, "%s", str)` where the return value is used to > `stpcpy(dest, str) - dest`. This generally avoids the machinery involved > in parsing format strings. This optimization was introduced into > clang-12. Because the kernel does not provide an implementation of > stpcpy, we observe linkage failures for almost all targets when building > with ToT clang. > > The interface is unsafe as it does not perform any bounds checking. > Disable this "libcall optimization" via `-fno-builtin-stpcpy`. > > Unlike > commit 5f074f3e192f ("lib/string.c: implement a basic bcmp") > which cited failures with `-fno-builtin-*` flags being retained in LLVM > LTO, that bug seems to have been fixed by > https://reviews.llvm.org/D71193, so the above sha can now be reverted in > favor of `-fno-builtin-bcmp`. > > Cc: stable@vger.kernel.org # 4.4 Why does a fix for Clang-12 have to be backported all the way to v4.4? How does that meet the requirements for stable patches? > Link: https://bugs.llvm.org/show_bug.cgi?id=47162 > Link: https://github.com/ClangBuiltLinux/linux/issues/1126 > Link: https://reviews.llvm.org/D85963 > Reported-by: Sami Tolvanen <samitolvanen@google.com> > Suggested-by: Dávid Bolvanský <david.bolvansky@gmail.com> > Suggested-by: Kees Cook <keescook@chromium.org> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > --- > Makefile | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Makefile b/Makefile > index 9cac6fde3479..211a1b6f6478 100644 > --- a/Makefile > +++ b/Makefile > @@ -959,6 +959,12 @@ ifdef CONFIG_RETPOLINE > KBUILD_CFLAGS += $(call cc-option,-fcf-protection=none) > endif > > +# The compiler may "libcall optimize" certain function calls into the below > +# functions, for architectures that don't use -ffreestanding. If we don't plan > +# to provide implementations of these routines, then prevent the compiler from > +# emitting calls to what will be undefined symbols. > +KBUILD_CFLAGS += -fno-builtin-stpcpy > + > # include additional Makefiles when needed > include-y := scripts/Makefile.extrawarn > include-$(CONFIG_KASAN) += scripts/Makefile.kasan > -- > 2.28.0.220.ged08abb693-goog >
On Tue, Aug 18, 2020 at 09:10:01AM +0200, Ard Biesheuvel wrote: > On Tue, 18 Aug 2020 at 00:02, Nick Desaulniers <ndesaulniers@google.com> wrote: > > > > LLVM implemented a recent "libcall optimization" that lowers calls to > > `sprintf(dest, "%s", str)` where the return value is used to > > `stpcpy(dest, str) - dest`. This generally avoids the machinery involved > > in parsing format strings. This optimization was introduced into > > clang-12. Because the kernel does not provide an implementation of > > stpcpy, we observe linkage failures for almost all targets when building > > with ToT clang. > > > > The interface is unsafe as it does not perform any bounds checking. > > Disable this "libcall optimization" via `-fno-builtin-stpcpy`. > > > > Unlike > > commit 5f074f3e192f ("lib/string.c: implement a basic bcmp") > > which cited failures with `-fno-builtin-*` flags being retained in LLVM > > LTO, that bug seems to have been fixed by > > https://reviews.llvm.org/D71193, so the above sha can now be reverted in > > favor of `-fno-builtin-bcmp`. > > > > Cc: stable@vger.kernel.org # 4.4 > > Why does a fix for Clang-12 have to be backported all the way to v4.4? > How does that meet the requirements for stable patches? Because people like to build older kernels with new compliler versions. And those "people" include me, who doesn't want to keep around old compilers just because my distro moved to the latest one... We've been doing this for the past 4+ years, for new versions of gcc, keeping 4.4.y building properly with the bleeding edge of that compiler, why is clang any different here? thanks, greg k-h
On Tue, 18 Aug 2020 at 09:25, Greg KH <gregkh@linuxfoundation.org> wrote: > > On Tue, Aug 18, 2020 at 09:10:01AM +0200, Ard Biesheuvel wrote: > > On Tue, 18 Aug 2020 at 00:02, Nick Desaulniers <ndesaulniers@google.com> wrote: > > > > > > LLVM implemented a recent "libcall optimization" that lowers calls to > > > `sprintf(dest, "%s", str)` where the return value is used to > > > `stpcpy(dest, str) - dest`. This generally avoids the machinery involved > > > in parsing format strings. This optimization was introduced into > > > clang-12. Because the kernel does not provide an implementation of > > > stpcpy, we observe linkage failures for almost all targets when building > > > with ToT clang. > > > > > > The interface is unsafe as it does not perform any bounds checking. > > > Disable this "libcall optimization" via `-fno-builtin-stpcpy`. > > > > > > Unlike > > > commit 5f074f3e192f ("lib/string.c: implement a basic bcmp") > > > which cited failures with `-fno-builtin-*` flags being retained in LLVM > > > LTO, that bug seems to have been fixed by > > > https://reviews.llvm.org/D71193, so the above sha can now be reverted in > > > favor of `-fno-builtin-bcmp`. > > > > > > Cc: stable@vger.kernel.org # 4.4 > > > > Why does a fix for Clang-12 have to be backported all the way to v4.4? > > How does that meet the requirements for stable patches? > > Because people like to build older kernels with new compliler versions. > > And those "people" include me, who doesn't want to keep around old > compilers just because my distro moved to the latest one... > > We've been doing this for the past 4+ years, for new versions of gcc, > keeping 4.4.y building properly with the bleeding edge of that compiler, > why is clang any different here? > Fair enough. I am just struggling to match stable-kernel-rules.rst with the actual practices - perhaps it is time to update that document?
On Tue, Aug 18, 2020 at 09:29:39AM +0200, Ard Biesheuvel wrote: > On Tue, 18 Aug 2020 at 09:25, Greg KH <gregkh@linuxfoundation.org> wrote: > > > > On Tue, Aug 18, 2020 at 09:10:01AM +0200, Ard Biesheuvel wrote: > > > On Tue, 18 Aug 2020 at 00:02, Nick Desaulniers <ndesaulniers@google.com> wrote: > > > > > > > > LLVM implemented a recent "libcall optimization" that lowers calls to > > > > `sprintf(dest, "%s", str)` where the return value is used to > > > > `stpcpy(dest, str) - dest`. This generally avoids the machinery involved > > > > in parsing format strings. This optimization was introduced into > > > > clang-12. Because the kernel does not provide an implementation of > > > > stpcpy, we observe linkage failures for almost all targets when building > > > > with ToT clang. > > > > > > > > The interface is unsafe as it does not perform any bounds checking. > > > > Disable this "libcall optimization" via `-fno-builtin-stpcpy`. > > > > > > > > Unlike > > > > commit 5f074f3e192f ("lib/string.c: implement a basic bcmp") > > > > which cited failures with `-fno-builtin-*` flags being retained in LLVM > > > > LTO, that bug seems to have been fixed by > > > > https://reviews.llvm.org/D71193, so the above sha can now be reverted in > > > > favor of `-fno-builtin-bcmp`. > > > > > > > > Cc: stable@vger.kernel.org # 4.4 > > > > > > Why does a fix for Clang-12 have to be backported all the way to v4.4? > > > How does that meet the requirements for stable patches? > > > > Because people like to build older kernels with new compliler versions. > > > > And those "people" include me, who doesn't want to keep around old > > compilers just because my distro moved to the latest one... > > > > We've been doing this for the past 4+ years, for new versions of gcc, > > keeping 4.4.y building properly with the bleeding edge of that compiler, > > why is clang any different here? > > > > Fair enough. I am just struggling to match stable-kernel-rules.rst > with the actual practices - perhaps it is time to update that > document? The rules are tiny and simple for 99% of the issues involved. Stuff like "add patches to fix build failures and warnings for newer compiler versions" are so rare (they only happen every 2 years or so), it's not worth it. thanks, greg k-h
On Mon, Aug 17, 2020 at 03:31:26PM -0700, H. Peter Anvin wrote: > On 2020-08-17 15:02, Nick Desaulniers wrote: > > LLVM implemented a recent "libcall optimization" that lowers calls to > > `sprintf(dest, "%s", str)` where the return value is used to > > `stpcpy(dest, str) - dest`. This generally avoids the machinery involved > > in parsing format strings. This optimization was introduced into > > clang-12. Because the kernel does not provide an implementation of > > stpcpy, we observe linkage failures for almost all targets when building > > with ToT clang. > > > > The interface is unsafe as it does not perform any bounds checking. > > Disable this "libcall optimization" via `-fno-builtin-stpcpy`. > > > > Unlike > > commit 5f074f3e192f ("lib/string.c: implement a basic bcmp") > > which cited failures with `-fno-builtin-*` flags being retained in LLVM > > LTO, that bug seems to have been fixed by > > https://reviews.llvm.org/D71193, so the above sha can now be reverted in > > favor of `-fno-builtin-bcmp`. > > > > stpcpy() and (to a lesser degree) mempcpy() are fairly useful routines > in general. Perhaps we *should* provide them? As Nick mentioned, I really don't want to expand the already bad interfaces from libc. We have enough messes to clean up already, and I don't want to add more. The kernel already uses a subset of C, we have (several) separate non-libc memory allocators, we're using strscpy() and scnprintf() widely in favor of their buggy libc counterparts, etc. We don't need to match the libc string interfaces especially when they're arguably bug-prone foot-guns. :)
On Mon, Aug 17, 2020 at 03:02:09PM -0700, Nick Desaulniers wrote: > LLVM implemented a recent "libcall optimization" that lowers calls to > `sprintf(dest, "%s", str)` where the return value is used to > `stpcpy(dest, str) - dest`. This generally avoids the machinery involved > in parsing format strings. This optimization was introduced into > clang-12. Because the kernel does not provide an implementation of > stpcpy, we observe linkage failures for almost all targets when building > with ToT clang. > > The interface is unsafe as it does not perform any bounds checking. > Disable this "libcall optimization" via `-fno-builtin-stpcpy`. > > Unlike > commit 5f074f3e192f ("lib/string.c: implement a basic bcmp") > which cited failures with `-fno-builtin-*` flags being retained in LLVM > LTO, that bug seems to have been fixed by > https://reviews.llvm.org/D71193, so the above sha can now be reverted in > favor of `-fno-builtin-bcmp`. > > Cc: stable@vger.kernel.org # 4.4 > Link: https://bugs.llvm.org/show_bug.cgi?id=47162 > Link: https://github.com/ClangBuiltLinux/linux/issues/1126 > Link: https://reviews.llvm.org/D85963 > Reported-by: Sami Tolvanen <samitolvanen@google.com> > Suggested-by: Dávid Bolvanský <david.bolvansky@gmail.com> > Suggested-by: Kees Cook <keescook@chromium.org> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> Reviewed-by: Kees Cook <keescook@chromium.org>
diff --git a/Makefile b/Makefile index 9cac6fde3479..211a1b6f6478 100644 --- a/Makefile +++ b/Makefile @@ -959,6 +959,12 @@ ifdef CONFIG_RETPOLINE KBUILD_CFLAGS += $(call cc-option,-fcf-protection=none) endif +# The compiler may "libcall optimize" certain function calls into the below +# functions, for architectures that don't use -ffreestanding. If we don't plan +# to provide implementations of these routines, then prevent the compiler from +# emitting calls to what will be undefined symbols. +KBUILD_CFLAGS += -fno-builtin-stpcpy + # include additional Makefiles when needed include-y := scripts/Makefile.extrawarn include-$(CONFIG_KASAN) += scripts/Makefile.kasan
LLVM implemented a recent "libcall optimization" that lowers calls to `sprintf(dest, "%s", str)` where the return value is used to `stpcpy(dest, str) - dest`. This generally avoids the machinery involved in parsing format strings. This optimization was introduced into clang-12. Because the kernel does not provide an implementation of stpcpy, we observe linkage failures for almost all targets when building with ToT clang. The interface is unsafe as it does not perform any bounds checking. Disable this "libcall optimization" via `-fno-builtin-stpcpy`. Unlike commit 5f074f3e192f ("lib/string.c: implement a basic bcmp") which cited failures with `-fno-builtin-*` flags being retained in LLVM LTO, that bug seems to have been fixed by https://reviews.llvm.org/D71193, so the above sha can now be reverted in favor of `-fno-builtin-bcmp`. Cc: stable@vger.kernel.org # 4.4 Link: https://bugs.llvm.org/show_bug.cgi?id=47162 Link: https://github.com/ClangBuiltLinux/linux/issues/1126 Link: https://reviews.llvm.org/D85963 Reported-by: Sami Tolvanen <samitolvanen@google.com> Suggested-by: Dávid Bolvanský <david.bolvansky@gmail.com> Suggested-by: Kees Cook <keescook@chromium.org> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- Makefile | 6 ++++++ 1 file changed, 6 insertions(+)