2023-08-06 08:40:46

by WANG Xuerui

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

From: WANG Xuerui <[email protected]>

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 <[email protected]>
Suggested-by: Nick Desaulniers <[email protected]>
Signed-off-by: WANG Xuerui <[email protected]>
---

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



2023-08-06 09:40:00

by Huacai Chen

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

Hi, Xuerui,

On Sun, Aug 6, 2023 at 4:30 PM WANG Xuerui <[email protected]> wrote:
>
> From: WANG Xuerui <[email protected]>
>
> 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 <[email protected]>
> Suggested-by: Nick Desaulniers <[email protected]>
> Signed-off-by: WANG Xuerui <[email protected]>
> ---
>
> 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
>
>

2023-08-06 11:08:42

by WANG Xuerui

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

Hi,

On 8/6/23 17:16, Huacai Chen wrote:
> Hi, Xuerui,
>
> On Sun, Aug 6, 2023 at 4:30 PM WANG Xuerui <[email protected]> wrote:
>> From: WANG Xuerui <[email protected]>
>>
>> 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 <[email protected]>
>> Suggested-by: Nick Desaulniers <[email protected]>
>> Signed-off-by: WANG Xuerui <[email protected]>
>> ---
>>
>> 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/


2023-08-07 08:24:48

by Xi Ruoyao

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

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 <[email protected]>
School of Aerospace Science and Technology, Xidian University

2023-08-07 17:12:17

by Nick Desaulniers

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

On Mon, Aug 7, 2023 at 12:17 AM Xi Ruoyao <[email protected]> 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