Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

r370454 breaks pseries_defconfig #647

Open
nathanchance opened this issue Sep 1, 2019 · 11 comments

Comments

@nickdesaulniers

This comment has been minimized.

Copy link
Member

commented Sep 3, 2019

  CALL    arch/powerpc/kernel/prom_init_check.sh
Error: External symbol 'bcmp' referenced from prom_init.c
make[2]: *** [arch/powerpc/kernel/Makefile:196: prom_init_check] Error 1

https://elixir.bootlin.com/linux/v5.3-rc7/source/arch/powerpc/kernel/prom_init_check.sh

# This script checks prom_init.o to see what external symbols it
# is using, if it finds symbols not in the whitelist it returns
# an error. The point of this is to discourage people from
# intentionally or accidentally adding new code to prom_init.c
# which has side effects on other parts of the kernel.

# If you really need to reference something from prom_init.o add
# it to the list below:

grep "^CONFIG_KASAN=y$" .config >/dev/null
if [ $? -eq 0 ]
then
	MEM_FUNCS="__memcpy __memset"
else
	MEM_FUNCS="memcpy memset"
fi

WHITELIST="add_reloc_offset __bss_start __bss_stop copy_and_flush
_end enter_prom $MEM_FUNCS reloc_offset __secondary_hold
__secondary_hold_acknowledge __secondary_hold_spinloop __start
logo_linux_clut224 btext_prepare_BAT
reloc_got2 kernstart_addr memstart_addr linux_banner _stext
__prom_init_toc_start __prom_init_toc_end btext_setup_display TOC."

So $MEM_FUNCS allows calls memcpy and memset via $WHITELIST. bcmp needs to be added to the list of $MEM_FUNCS.

cc @ramosian-glider if recent bcmp optimizations in LLVM have implications for KASAN, ie, the double underscored prefixed versions of these functions.

@nathanchance

This comment has been minimized.

Copy link
Member Author

commented Sep 4, 2019

Fair enough, wasn't sure if there was any reason not to do that. I can send along a patch that adds bcmp to the whitelist (I'll try to do this tomorrow).

@nathanchance

This comment has been minimized.

Copy link
Member Author

commented Sep 5, 2019

Looking into this further, it looks like prom_init.c has its own set of mem* and str* functions (including a version of memcmp): torvalds@450e7dd

I am not sure that allowing a reference to the generic bcmp function in lib/string.c will be allowed (although prom_memcmp is the same as the generic memcmp so it should be fine)...? It appears in prom_strstr according to the disassembly, I assume because of the looping with memcmp; however, this type of transformation does not seem beneficial for the kernel at all because our bcmp is just a wrapper around memcmp (so we are going to go from memcmp to bcmp back to memcmp but I might be misunderstanding this whole transformation thing).

This could be solved tangentially by adding -ffreestanding to arch/powerpc, which was potentially suggested as a solution for #625: https://lore.kernel.org/linuxppc-dev/20190904130135.GN9749@gate.crashing.org/

@nickdesaulniers

This comment has been minimized.

Copy link
Member

commented Sep 5, 2019

problem is that the compiler is going to insert calls to bcmp, so you can't define prom_bcmp. (Compiler will also insert calls to memset/memcpy, too). So -ffreestanding is probably a better fix (at least for prom).

@nathanchance

This comment has been minimized.

Copy link
Member Author

commented Sep 9, 2019

Looks like the problematic LLVM commit was reverted almost a week ago so it doesn't appear to be a high priority to fix this right away: llvm/llvm-project@bdd6535

@nickdesaulniers

This comment has been minimized.

Copy link
Member

commented Sep 9, 2019

Bug: https://bugs.llvm.org/show_bug.cgi?id=43206.
Let's close this bug and reopen should the problem come back. Since it sounds like the problematic commit to llvm has been reverted, I'm not sure there's more to track here just yet.

@nickdesaulniers

This comment has been minimized.

Copy link
Member

commented Sep 9, 2019

@nathanchance

This comment has been minimized.

Copy link
Member Author

commented Sep 9, 2019

I can probably draft up a patch (already tentatively have one: https://gist.github.com/nathanchance/0472b44f1403e98c4a10bdc5e46d2d0b) but I feel like my PowerPC patches end up going nowhere...

@nickdesaulniers

This comment has been minimized.

Copy link
Member

commented Sep 9, 2019

but I feel like my PowerPC patches end up going nowhere...

Life is meaningless, so don't sweat the small stuff. 👹

That patch LGTM (I'd sign off when posted to the list)

@nathanchance

This comment has been minimized.

@nathanchance nathanchance self-assigned this Sep 11, 2019
nathanchance added a commit to nathanchance/linux that referenced this issue Oct 10, 2019
r370454 gives LLVM the ability to convert certain loops into a reference
to bcmp as an optimization; this breaks prom_init_check.sh:

  CALL    arch/powerpc/kernel/prom_init_check.sh
Error: External symbol 'bcmp' referenced from prom_init.c
make[2]: *** [arch/powerpc/kernel/Makefile:196: prom_init_check] Error 1

bcmp is defined in lib/string.c as a wrapper for memcmp so this could be
added to the whitelist. However, commit 450e7dd ("powerpc/prom_init:
don't use string functions from lib/") copied memcmp as prom_memcmp to
avoid KASAN instrumentation so having bcmp be resolved to regular memcmp
would break that assumption. Furthermore, because the compiler is the
one that inserted bcmp, we cannot provide something like prom_bcmp.

To prevent LLVM from being clever with optimizations like this, use
-ffreestanding to tell LLVM we are not hosted so it is not free to make
transformations like this.

Link: ClangBuiltLinux#647
Link: llvm/llvm-project@5c9f3cf
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
(am from https://lore.kernel.org/linuxppc-dev/20190911182049.77853-4-natechancellor@gmail.com/)
nathanchance added a commit to nathanchance/linux that referenced this issue Oct 10, 2019
r370454 gives LLVM the ability to convert certain loops into a reference
to bcmp as an optimization; this breaks prom_init_check.sh:

  CALL    arch/powerpc/kernel/prom_init_check.sh
Error: External symbol 'bcmp' referenced from prom_init.c
make[2]: *** [arch/powerpc/kernel/Makefile:196: prom_init_check] Error 1

bcmp is defined in lib/string.c as a wrapper for memcmp so this could be
added to the whitelist. However, commit 450e7dd ("powerpc/prom_init:
don't use string functions from lib/") copied memcmp as prom_memcmp to
avoid KASAN instrumentation so having bcmp be resolved to regular memcmp
would break that assumption. Furthermore, because the compiler is the
one that inserted bcmp, we cannot provide something like prom_bcmp.

To prevent LLVM from being clever with optimizations like this, use
-ffreestanding to tell LLVM we are not hosted so it is not free to make
transformations like this.

Link: ClangBuiltLinux#647
Link: llvm/llvm-project@5c9f3cf
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
nathanchance added a commit to nathanchance/linux that referenced this issue Oct 14, 2019
r370454 gives LLVM the ability to convert certain loops into a reference
to bcmp as an optimization; this breaks prom_init_check.sh:

  CALL    arch/powerpc/kernel/prom_init_check.sh
Error: External symbol 'bcmp' referenced from prom_init.c
make[2]: *** [arch/powerpc/kernel/Makefile:196: prom_init_check] Error 1

bcmp is defined in lib/string.c as a wrapper for memcmp so this could be
added to the whitelist. However, commit 450e7dd ("powerpc/prom_init:
don't use string functions from lib/") copied memcmp as prom_memcmp to
avoid KASAN instrumentation so having bcmp be resolved to regular memcmp
would break that assumption. Furthermore, because the compiler is the
one that inserted bcmp, we cannot provide something like prom_bcmp.

To prevent LLVM from being clever with optimizations like this, use
-ffreestanding to tell LLVM we are not hosted so it is not free to make
transformations like this.

Link: ClangBuiltLinux#647
Link: llvm/llvm-project@5c9f3cf
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
(am from https://lore.kernel.org/linuxppc-dev/20190911182049.77853-4-natechancellor@gmail.com/)
nathanchance added a commit to nathanchance/linux that referenced this issue Oct 14, 2019
r374662 gives LLVM the ability to convert certain loops into a reference
to bcmp as an optimization; this breaks prom_init_check.sh:

  CALL    arch/powerpc/kernel/prom_init_check.sh
Error: External symbol 'bcmp' referenced from prom_init.c
make[2]: *** [arch/powerpc/kernel/Makefile:196: prom_init_check] Error 1

bcmp is defined in lib/string.c as a wrapper for memcmp so this could be
added to the whitelist. However, commit 450e7dd ("powerpc/prom_init:
don't use string functions from lib/") copied memcmp as prom_memcmp to
avoid KASAN instrumentation so having bcmp be resolved to regular memcmp
would break that assumption. Furthermore, because the compiler is the
one that inserted bcmp, we cannot provide something like prom_bcmp.

To prevent LLVM from being clever with optimizations like this, use
-ffreestanding to tell LLVM we are not hosted so it is not free to make
transformations like this.

Link: ClangBuiltLinux#647
Link: llvm/llvm-project@76cdcf2
Reviewed-by: Nick Desaulneris <ndesaulniers@google.com>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Link: https://lore.kernel.org/lkml/20191014025101.18567-4-natechancellor@gmail.com/
ruscur pushed a commit to ruscur/linux that referenced this issue Oct 14, 2019
r374662 gives LLVM the ability to convert certain loops into a reference
to bcmp as an optimization; this breaks prom_init_check.sh:

  CALL    arch/powerpc/kernel/prom_init_check.sh
Error: External symbol 'bcmp' referenced from prom_init.c
make[2]: *** [arch/powerpc/kernel/Makefile:196: prom_init_check] Error 1

bcmp is defined in lib/string.c as a wrapper for memcmp so this could be
added to the whitelist. However, commit 450e7dd ("powerpc/prom_init:
don't use string functions from lib/") copied memcmp as prom_memcmp to
avoid KASAN instrumentation so having bcmp be resolved to regular memcmp
would break that assumption. Furthermore, because the compiler is the
one that inserted bcmp, we cannot provide something like prom_bcmp.

To prevent LLVM from being clever with optimizations like this, use
-ffreestanding to tell LLVM we are not hosted so it is not free to make
transformations like this.

Link: ClangBuiltLinux#647
Link: llvm/llvm-project@76cdcf2
Reviewed-by: Nick Desaulneris <ndesaulniers@google.com>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
fengguang pushed a commit to 0day-ci/linux that referenced this issue Oct 14, 2019
r374662 gives LLVM the ability to convert certain loops into a reference
to bcmp as an optimization; this breaks prom_init_check.sh:

  CALL    arch/powerpc/kernel/prom_init_check.sh
Error: External symbol 'bcmp' referenced from prom_init.c
make[2]: *** [arch/powerpc/kernel/Makefile:196: prom_init_check] Error 1

bcmp is defined in lib/string.c as a wrapper for memcmp so this could be
added to the whitelist. However, commit 450e7dd ("powerpc/prom_init:
don't use string functions from lib/") copied memcmp as prom_memcmp to
avoid KASAN instrumentation so having bcmp be resolved to regular memcmp
would break that assumption. Furthermore, because the compiler is the
one that inserted bcmp, we cannot provide something like prom_bcmp.

To prevent LLVM from being clever with optimizations like this, use
-ffreestanding to tell LLVM we are not hosted so it is not free to make
transformations like this.

Link: ClangBuiltLinux#647
Link: llvm/llvm-project@76cdcf2
Reviewed-by: Nick Desaulneris <ndesaulniers@google.com>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
@LebedevRI

This comment has been minimized.

Copy link

commented Oct 14, 2019

Sorry, didn't track this issue here, it is good that a retroactive fix was attempted.

daxtens added a commit to daxtens/linux that referenced this issue Oct 15, 2019
r374662 gives LLVM the ability to convert certain loops into a reference
to bcmp as an optimization; this breaks prom_init_check.sh:

  CALL    arch/powerpc/kernel/prom_init_check.sh
Error: External symbol 'bcmp' referenced from prom_init.c
make[2]: *** [arch/powerpc/kernel/Makefile:196: prom_init_check] Error 1

bcmp is defined in lib/string.c as a wrapper for memcmp so this could be
added to the whitelist. However, commit 450e7dd ("powerpc/prom_init:
don't use string functions from lib/") copied memcmp as prom_memcmp to
avoid KASAN instrumentation so having bcmp be resolved to regular memcmp
would break that assumption. Furthermore, because the compiler is the
one that inserted bcmp, we cannot provide something like prom_bcmp.

To prevent LLVM from being clever with optimizations like this, use
-ffreestanding to tell LLVM we are not hosted so it is not free to make
transformations like this.

Link: ClangBuiltLinux#647
Link: llvm/llvm-project@76cdcf2
Reviewed-by: Nick Desaulneris <ndesaulniers@google.com>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
ruscur pushed a commit to ruscur/linux that referenced this issue Oct 16, 2019
r374662 gives LLVM the ability to convert certain loops into a reference
to bcmp as an optimization; this breaks prom_init_check.sh:

  CALL    arch/powerpc/kernel/prom_init_check.sh
Error: External symbol 'bcmp' referenced from prom_init.c
make[2]: *** [arch/powerpc/kernel/Makefile:196: prom_init_check] Error 1

bcmp is defined in lib/string.c as a wrapper for memcmp so this could be
added to the whitelist. However, commit 450e7dd ("powerpc/prom_init:
don't use string functions from lib/") copied memcmp as prom_memcmp to
avoid KASAN instrumentation so having bcmp be resolved to regular memcmp
would break that assumption. Furthermore, because the compiler is the
one that inserted bcmp, we cannot provide something like prom_bcmp.

To prevent LLVM from being clever with optimizations like this, use
-ffreestanding to tell LLVM we are not hosted so it is not free to make
transformations like this.

Link: ClangBuiltLinux#647
Link: llvm/llvm-project@76cdcf2
Reviewed-by: Nick Desaulneris <ndesaulniers@google.com>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
ruscur pushed a commit to ruscur/linux3 that referenced this issue Oct 16, 2019
r374662 gives LLVM the ability to convert certain loops into a reference
to bcmp as an optimization; this breaks prom_init_check.sh:

  CALL    arch/powerpc/kernel/prom_init_check.sh
Error: External symbol 'bcmp' referenced from prom_init.c
make[2]: *** [arch/powerpc/kernel/Makefile:196: prom_init_check] Error 1

bcmp is defined in lib/string.c as a wrapper for memcmp so this could be
added to the whitelist. However, commit 450e7dd ("powerpc/prom_init:
don't use string functions from lib/") copied memcmp as prom_memcmp to
avoid KASAN instrumentation so having bcmp be resolved to regular memcmp
would break that assumption. Furthermore, because the compiler is the
one that inserted bcmp, we cannot provide something like prom_bcmp.

To prevent LLVM from being clever with optimizations like this, use
-ffreestanding to tell LLVM we are not hosted so it is not free to make
transformations like this.

Link: ClangBuiltLinux/linux#647
Link: llvm/llvm-project@76cdcf2
Reviewed-by: Nick Desaulneris <ndesaulniers@google.com>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
ruscur pushed a commit to ruscur/linux that referenced this issue Oct 16, 2019
r374662 gives LLVM the ability to convert certain loops into a reference
to bcmp as an optimization; this breaks prom_init_check.sh:

  CALL    arch/powerpc/kernel/prom_init_check.sh
Error: External symbol 'bcmp' referenced from prom_init.c
make[2]: *** [arch/powerpc/kernel/Makefile:196: prom_init_check] Error 1

bcmp is defined in lib/string.c as a wrapper for memcmp so this could be
added to the whitelist. However, commit 450e7dd ("powerpc/prom_init:
don't use string functions from lib/") copied memcmp as prom_memcmp to
avoid KASAN instrumentation so having bcmp be resolved to regular memcmp
would break that assumption. Furthermore, because the compiler is the
one that inserted bcmp, we cannot provide something like prom_bcmp.

To prevent LLVM from being clever with optimizations like this, use
-ffreestanding to tell LLVM we are not hosted so it is not free to make
transformations like this.

Link: ClangBuiltLinux#647
Link: llvm/llvm-project@76cdcf2
Reviewed-by: Nick Desaulneris <ndesaulniers@google.com>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.