[PATCH v2] LoongArch: Replace -ffreestanding with finer-grained -fno-builtin's

WANG Xuerui posted 1 patch 2 years, 4 months ago
There is a newer version of this series
arch/loongarch/Makefile | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
[PATCH v2] LoongArch: Replace -ffreestanding with finer-grained -fno-builtin's
Posted by WANG Xuerui 2 years, 4 months ago
From: WANG Xuerui <git@xen0n.name>

As explained by Nick in the original issue: the kernel usually does a
good job of providing library helpers that have similar semantics as
their ordinary userspace libc equivalents, but -ffreestanding disables
such libcall optimization and other related features in the compiler,
which can lead to unexpected things such as CONFIG_FORTIFY_SOURCE not
working (!).

As it turns out to be the case, only the memory operations really need
to be prevented from expansion by the compiler, and this is doable with
finer-grained -fno-builtin-* toggles. So only disable memcpy, memmove
and memset, while leaving other builtins enabled, to fix source
fortification among others.

Closes: https://github.com/ClangBuiltLinux/linux/issues/1897
Reported-by: Nathan Chancellor <nathan@kernel.org>
Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: WANG Xuerui <git@xen0n.name>
---

Changes in v2:

- Keep the memory operation builtins disabled, add comments, and tweak the
  commit message along the way.

 arch/loongarch/Makefile | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
index b1e5db51b61c..34fc48df87f2 100644
--- a/arch/loongarch/Makefile
+++ b/arch/loongarch/Makefile
@@ -83,7 +83,14 @@ KBUILD_CFLAGS_KERNEL		+= -fPIE
 LDFLAGS_vmlinux			+= -static -pie --no-dynamic-linker -z notext
 endif
 
-cflags-y += -ffreestanding
+# Make sure the memory libcalls are not expanded by the compiler, for better
+# control over unaligned accesses with respect to CONFIG_ARCH_STRICT_ALIGN,
+# and also for avoiding https://gcc.gnu.org/PR109465.
+#
+# The overly broad -ffreestanding is undesirable as it disables *all* libcall
+# handling, that unfortunately includes proper FORTIFY_SOURCE instrumentation.
+cflags-y += -fno-builtin-memcpy -fno-builtin-memmove -fno-builtin-memset
+
 cflags-y += $(call cc-option, -mno-check-zero-division)
 
 load-y		= 0x9000000000200000
-- 
2.40.0
Re: [PATCH v2] LoongArch: Replace -ffreestanding with finer-grained -fno-builtin's
Posted by Huacai Chen 2 years, 4 months ago
Hi, Xuerui,

On Sun, Aug 6, 2023 at 4:30 PM WANG Xuerui <kernel@xen0n.name> wrote:
>
> From: WANG Xuerui <git@xen0n.name>
>
> As explained by Nick in the original issue: the kernel usually does a
> good job of providing library helpers that have similar semantics as
> their ordinary userspace libc equivalents, but -ffreestanding disables
> such libcall optimization and other related features in the compiler,
> which can lead to unexpected things such as CONFIG_FORTIFY_SOURCE not
> working (!).
>
> As it turns out to be the case, only the memory operations really need
> to be prevented from expansion by the compiler, and this is doable with
> finer-grained -fno-builtin-* toggles. So only disable memcpy, memmove
> and memset, while leaving other builtins enabled, to fix source
> fortification among others.
>
> Closes: https://github.com/ClangBuiltLinux/linux/issues/1897
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: WANG Xuerui <git@xen0n.name>
> ---
>
> Changes in v2:
>
> - Keep the memory operation builtins disabled, add comments, and tweak the
>   commit message along the way.
>
>  arch/loongarch/Makefile | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
> index b1e5db51b61c..34fc48df87f2 100644
> --- a/arch/loongarch/Makefile
> +++ b/arch/loongarch/Makefile
> @@ -83,7 +83,14 @@ KBUILD_CFLAGS_KERNEL         += -fPIE
>  LDFLAGS_vmlinux                        += -static -pie --no-dynamic-linker -z notext
>  endif
>
> -cflags-y += -ffreestanding
> +# Make sure the memory libcalls are not expanded by the compiler, for better
> +# control over unaligned accesses with respect to CONFIG_ARCH_STRICT_ALIGN,
> +# and also for avoiding https://gcc.gnu.org/PR109465.
> +#
> +# The overly broad -ffreestanding is undesirable as it disables *all* libcall
> +# handling, that unfortunately includes proper FORTIFY_SOURCE instrumentation.
I think these comments should go to commit message rather than here,
because after this patch there is no -ffreestanding in Makefile.

Huacai

> +cflags-y += -fno-builtin-memcpy -fno-builtin-memmove -fno-builtin-memset
> +
>  cflags-y += $(call cc-option, -mno-check-zero-division)
>
>  load-y         = 0x9000000000200000
> --
> 2.40.0
>
>
Re: [PATCH v2] LoongArch: Replace -ffreestanding with finer-grained -fno-builtin's
Posted by WANG Xuerui 2 years, 4 months ago
Hi,

On 8/6/23 17:16, Huacai Chen wrote:
> Hi, Xuerui,
>
> On Sun, Aug 6, 2023 at 4:30 PM WANG Xuerui <kernel@xen0n.name> wrote:
>> From: WANG Xuerui <git@xen0n.name>
>>
>> As explained by Nick in the original issue: the kernel usually does a
>> good job of providing library helpers that have similar semantics as
>> their ordinary userspace libc equivalents, but -ffreestanding disables
>> such libcall optimization and other related features in the compiler,
>> which can lead to unexpected things such as CONFIG_FORTIFY_SOURCE not
>> working (!).
>>
>> As it turns out to be the case, only the memory operations really need
>> to be prevented from expansion by the compiler, and this is doable with
>> finer-grained -fno-builtin-* toggles. So only disable memcpy, memmove
>> and memset, while leaving other builtins enabled, to fix source
>> fortification among others.
>>
>> Closes: https://github.com/ClangBuiltLinux/linux/issues/1897
>> Reported-by: Nathan Chancellor <nathan@kernel.org>
>> Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
>> Signed-off-by: WANG Xuerui <git@xen0n.name>
>> ---
>>
>> Changes in v2:
>>
>> - Keep the memory operation builtins disabled, add comments, and tweak the
>>    commit message along the way.
>>
>>   arch/loongarch/Makefile | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
>> index b1e5db51b61c..34fc48df87f2 100644
>> --- a/arch/loongarch/Makefile
>> +++ b/arch/loongarch/Makefile
>> @@ -83,7 +83,14 @@ KBUILD_CFLAGS_KERNEL         += -fPIE
>>   LDFLAGS_vmlinux                        += -static -pie --no-dynamic-linker -z notext
>>   endif
>>
>> -cflags-y += -ffreestanding
>> +# Make sure the memory libcalls are not expanded by the compiler, for better
>> +# control over unaligned accesses with respect to CONFIG_ARCH_STRICT_ALIGN,
>> +# and also for avoiding https://gcc.gnu.org/PR109465.
>> +#
>> +# The overly broad -ffreestanding is undesirable as it disables *all* libcall
>> +# handling, that unfortunately includes proper FORTIFY_SOURCE instrumentation.
> I think these comments should go to commit message rather than here,
> because after this patch there is no -ffreestanding in Makefile.
Thanks for the advice, I'm fine either way and I'll send v3.
>
> Huacai
>
>> +cflags-y += -fno-builtin-memcpy -fno-builtin-memmove -fno-builtin-memset
>> +
>>   cflags-y += $(call cc-option, -mno-check-zero-division)
>>
>>   load-y         = 0x9000000000200000
>> --
>> 2.40.0
>>
>>
-- 
WANG "xen0n" Xuerui

Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/

Re: [PATCH v2] LoongArch: Replace -ffreestanding with finer-grained -fno-builtin's
Posted by Xi Ruoyao 2 years, 4 months ago
On Sun, 2023-08-06 at 18:31 +0800, WANG Xuerui wrote:
> > > +cflags-y += -fno-builtin-memcpy -fno-builtin-memmove -fno-builtin-memset

Can we only apply them for GCC <= 13?  I think with GCC 14 the built-in
implementations are quite reasonable (well, maybe it's some human ego
because I wrote them :).

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University
Re: [PATCH v2] LoongArch: Replace -ffreestanding with finer-grained -fno-builtin's
Posted by Nick Desaulniers 2 years, 4 months ago
On Mon, Aug 7, 2023 at 12:17 AM Xi Ruoyao <xry111@xry111.site> wrote:
>
> On Sun, 2023-08-06 at 18:31 +0800, WANG Xuerui wrote:
> > > > +cflags-y += -fno-builtin-memcpy -fno-builtin-memmove -fno-builtin-memset
>
> Can we only apply them for GCC <= 13?  I think with GCC 14 the built-in
> implementations are quite reasonable (well, maybe it's some human ego
> because I wrote them :).

Yes, and because only GCC-13 and older users should pay that penalty.

-- 
Thanks,
~Nick Desaulniers