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.

Sign up for GitHub

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

Jump to bottom

changed binding to STB_GLOBAL #1190

Closed
nickdesaulniers opened this issue Nov 4, 2020 · 11 comments
Closed

changed binding to STB_GLOBAL #1190

nickdesaulniers opened this issue Nov 4, 2020 · 11 comments
Assignees
@MaskRay
Labels
[ARCH] arm32 This bug impacts ARCH=arm [ARCH] arm64 This bug impacts ARCH=arm64 [ARCH] x86_64 This bug impacts ARCH=x86_64 [BUG] linux A bug that should be fixed in the mainline kernel. [FIXED][LINUX] 5.11 This bug was fixed in Linux 5.11 [TOOL] integrated-as The issue is relevant to LLVM integrated assembler

Comments

@nickdesaulniers
Copy link
Member

https://reviews.llvm.org/D90108 broke LLVM_IAS=1 for x86_64, arm64, and arm. Filing an issue to track the fixes, since we'll need to track they all get fixed and backported to 4.19.

cc @MaskRay @jcai19 @samitolvanen

x86 patch is accepted: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=4d6ffa27b8e5116c0abb318790fd01d4e12d75e6
arm64 patch is in mainline: ec9d780
perf patch submitted: https://lore.kernel.org/lkml/20201104005609.1316230-1-maskray@google.com/

I think 32b arm is still a problem:

$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 LLVM_IAS=1 -j71 
  AS      arch/arm/lib/memcpy.o
  AS      arch/arm/lib/memmove.o
  AS      arch/arm/lib/memset.o
<unknown>:0: error: memcpy changed binding to STB_GLOBAL
make[1]: *** [scripts/Makefile.build:364: arch/arm/lib/memcpy.o] Error 1
make[1]: *** Waiting for unfinished jobs....
<unknown>:0: error: memmove changed binding to STB_GLOBAL
<unknown>:0: error: memset changed binding to STB_GLOBAL
@nickdesaulniers nickdesaulniers added [BUG] linux A bug that should be fixed in the mainline kernel. [TOOL] integrated-as The issue is relevant to LLVM integrated assembler [ARCH] arm64 This bug impacts ARCH=arm64 [ARCH] x86_64 This bug impacts ARCH=x86_64 [ARCH] arm32 This bug impacts ARCH=arm [PATCH] Submitted A patch has been submitted for review [PATCH] Accepted A submitted patch has been accepted upstream labels Nov 4, 2020
@MaskRay
Copy link
Member

MaskRay commented Nov 4, 2020

I'll start fixing arm now...

@nickdesaulniers
Copy link
Member Author

The 32b ARM failure is currently linux-next specific, due to this commit: https://lore.kernel.org/linux-arm-kernel/20201019084140.4532-3-linus.walleij@linaro.org/

@MaskRay
Copy link
Member

MaskRay commented Nov 5, 2020 •
edited

I don't know how to reference a commit in linux-next but not in mainline...
Just replied to "[PATCH 0/5 v16] KASan for Arm" by Linus Walleij (I could reply because @nathanchance kindly CCed it to clang-built-linux...)

@nathanchance
Copy link
Member

nathanchance commented Nov 5, 2020 •
edited

I don't know how to reference a commit in linux-next but not in mainline...

Same way:

Fixes: <sha> ("<commit title>")

If the maintainer's tree is not stable (i.e. they rebase), they will usually squash the fix in or they will need to adjust the SHA for the fixes commit. If it is the former, the tag won't matter regardless and if it is the latter, it is their burden to fix up the SHA in the Fixes tag, not yours (because there is nothing you can do).

The vast majority of maintainers will not rebase so their SHAs are constant.

@MaskRay
Copy link
Member

MaskRay commented Nov 5, 2020 •
edited by nickdesaulniers

Thanks:)

Sent [PATCH] ARM: Change arch/arm/lib/mem*.S to use WEAK instead of .weak

https://www.armlinux.org.uk/developer/patches/viewpatch.php?id=9022/1

@nathanchance
Copy link
Member

x86_64 patch has been merged into mainline: https://git.kernel.org/linus/4d6ffa27b8e5116c0abb318790fd01d4e12d75e6

sudipm-mukherjee pushed a commit to sudipm-mukherjee/linux-test that referenced this issue Nov 13, 2020
@MaskRay
ARM: 9022/1: Change arch/arm/lib/mem*.S to use WEAK instead of .weak
735e8d9 
Commit d6d51a9 ("ARM: 9014/2: Replace string mem* functions for
KASan") add .weak directives to memcpy/memmove/memset to avoid collision
with KASAN interceptors.

This does not work with LLVM's integrated assembler (the assembly snippet
`.weak memcpy ... .globl memcpy` produces a STB_GLOBAL memcpy while GNU as
produces a STB_WEAK memcpy). LLVM 12 (since https://reviews.llvm.org/D90108)
will error on such an overridden symbol binding.

Use the appropriate WEAK macro instead.

Link: ClangBuiltLinux/linux#1190
--

Fixes: d6d51a9 ("ARM: 9014/2: Replace string mem* functions for KASan")
Reported-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Fangrui Song <maskray@google.com>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
@nickdesaulniers
Copy link
Member Author

perf patch: https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=tmp.perf/urgent&id=db1a8b97a0a36155171dbb805fbcb276e07559f6

lazerl0rd added a commit to lazerl0rd/tryme_redbull that referenced this issue Dec 7, 2020
@lazerl0rd
arm64: Adjust mem*.S to use WEAK instead of .weak.
e1b1da1 
Commit 39d114d added .weak directives to
support KASAN on AArch64, however this breaks support for LLVM's
integrated assembler as LLVM switched the .weak directive to produce a
STB_GLOBAL binding rather than STB_WEAK [1].

By utilising the WEAK macro instead, both LLVM and GCC will produce a
STB_WEAK binding.

[1] : https://reviews.llvm.org/D90108

Link: ClangBuiltLinux/linux#1190
Signed-off-by: Diab Neiroukh <lazerl0rd@thezest.dev>
@nickdesaulniers nickdesaulniers removed the [PATCH] Submitted A patch has been submitted for review label Dec 8, 2020
@nickdesaulniers
Copy link
Member Author

backports sent for x86_64 on 5.4 and 4.19: https://lore.kernel.org/stable/CAKwvOdkK1LgLC4ChptzUTC45WvE9-Sn0OqtgF7-odNSw8xLTYA@mail.gmail.com/T/#u

@nickdesaulniers nickdesaulniers added the Needs Backport Should be backported to either linux-stable tree or latest llvm release branch. label Dec 9, 2020
@lazerl0rd
Copy link
Member

backports sent for x86_64 on 5.4 and 4.19: https://lore.kernel.org/stable/CAKwvOdkK1LgLC4ChptzUTC45WvE9-Sn0OqtgF7-odNSw8xLTYA@mail.gmail.com/T/#u

I'll drop my temporary Linux 4.19 patch and merge this to avoid confusion.

@nickdesaulniers nickdesaulniers removed the Needs Backport Should be backported to either linux-stable tree or latest llvm release branch. label Dec 11, 2020
@nickdesaulniers
Copy link
Member Author

arm64 patch ec9d780 landed in v5.10-rc2.
63632bd in v5.9.5.
ca16a42 in v5.4.76.
DNE in v4.19.y.

conflicts: 3ac0f45 v5.6-rc1
35e61c7 v5.6-rc1

backport sent: https://groups.google.com/g/clang-built-linux/c/jHD0eQXFPGk

@nickdesaulniers nickdesaulniers mentioned this issue Jan 27, 2021
Closed
it-is-a-robot pushed a commit to openeuler-mirror/kernel that referenced this issue Feb 11, 2021
@MaskRay
ARM: 9022/1: Change arch/arm/lib/mem*.S to use WEAK instead of .weak
4c88513 
mainline inclusion
from mainline-5.11-rc1
commit 735e8d9
category: feature
feature: ARM KASAN support
bugzilla: 46872
CVE: NA

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=735e8d93dc2b107f7891a9c2b1c4cfbea1fcbbbc

-------------------------------------------------
Commit d6d51a9 ("ARM: 9014/2: Replace string mem* functions for
KASan") add .weak directives to memcpy/memmove/memset to avoid collision
with KASAN interceptors.

This does not work with LLVM's integrated assembler (the assembly snippet
`.weak memcpy ... .globl memcpy` produces a STB_GLOBAL memcpy while GNU as
produces a STB_WEAK memcpy). LLVM 12 (since https://reviews.llvm.org/D90108)
will error on such an overridden symbol binding.

Use the appropriate WEAK macro instead.

Link: ClangBuiltLinux/linux#1190
Reviewed-by: Jing Xiangfeng <jingxiangfeng@huawei.com>
Signed-off-by: Zheng Zengkai <zhengzengkai@huawei.com>
@nickdesaulniers
Copy link
Member Author

arm: 735e8d9 (v5.11-rc1)
arm64: ec9d780 (v5.10-rc2)
x86_64: 4d6ffa2 (v5.10-rc3)
perf: db1a8b9 (v5.10-rc5)

@nickdesaulniers nickdesaulniers removed the [PATCH] Accepted A submitted patch has been accepted upstream label Feb 22, 2021
@nickdesaulniers nickdesaulniers added the [FIXED][LINUX] 5.11 This bug was fixed in Linux 5.11 label Feb 22, 2021
laiyoufafa pushed a commit to laiyoufafa/kernel_linux_5.10 that referenced this issue Apr 13, 2023
@MaskRay @yuchangchun1
ARM: 9022/1: Change arch/arm/lib/mem*.S to use WEAK instead of .weak
e0921c0 
mainline inclusion
from mainline-5.11-rc1
commit 735e8d93dc2b107f7891a9c2b1c4cfbea1fcbbbc
category: feature
feature: ARM KASAN support
issue: #I3ZXZF
CVE: NA

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=735e8d93dc2b107f7891a9c2b1c4cfbea1fcbbbc

-------------------------------------------------
Commit d6d51a96c7d6 ("ARM: 9014/2: Replace string mem* functions for
KASan") add .weak directives to memcpy/memmove/memset to avoid collision
with KASAN interceptors.

This does not work with LLVM's integrated assembler (the assembly snippet
`.weak memcpy ... .globl memcpy` produces a STB_GLOBAL memcpy while GNU as
produces a STB_WEAK memcpy). LLVM 12 (since https://reviews.llvm.org/D90108)
will error on such an overridden symbol binding.

Use the appropriate WEAK macro instead.

Link: ClangBuiltLinux/linux#1190
Reviewed-by: Jing Xiangfeng <jingxiangfeng@huawei.com>
Signed-off-by: Chen Jun <chenjun102@huawei.com>
Signed-off-by: Yu Changchun <yuchangchun1@huawei.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@MaskRay MaskRay

Labels
[ARCH] arm32 This bug impacts ARCH=arm [ARCH] arm64 This bug impacts ARCH=arm64 [ARCH] x86_64 This bug impacts ARCH=x86_64 [BUG] linux A bug that should be fixed in the mainline kernel. [FIXED][LINUX] 5.11 This bug was fixed in Linux 5.11 [TOOL] integrated-as The issue is relevant to LLVM integrated assembler
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@MaskRay @nickdesaulniers @nathanchance @lazerl0rd