[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

https://github.com/phoebewang closed https://github.com/llvm/llvm-project/pull/121070 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

phoebewang wrote: > @lvwr @maurer @rcvalle A gentle reminder to please review this PR. @scottconstable You don't need explicit approvals from all reviewers. Let's wait for 24 hours and land it if no objections. https://github.com/llvm/llvm-project/pull/121070 __

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

scottconstable wrote: @lvwr @maurer @rcvalle A gentle reminder to please review this PR. https://github.com/llvm/llvm-project/pull/121070 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

https://github.com/phoebewang approved this pull request. LGTM. https://github.com/llvm/llvm-project/pull/121070 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

https://github.com/samitolvanen approved this pull request. Thank you for adding the command line flag. I agree that this should be sufficient to address compatibility concerns. https://github.com/llvm/llvm-project/pull/121070 ___ cfe-commits mailing

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

https://github.com/scottconstable updated https://github.com/llvm/llvm-project/pull/121070 >From 54296a3991e186629889611eff9e7cdd2c1cadca Mon Sep 17 00:00:00 2001 From: Scott D Constable Date: Mon, 23 Dec 2024 13:48:48 -0800 Subject: [PATCH] Implement a new kcfi_arity feature that encodes an in

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

samitolvanen wrote: > And AFAIK there's no other users of kCFI besides the Linux kernel (but please > correct me if I'm wrong). There are firmware projects that use KCFI, but AFAIK none of them run on x86. I'm also fairly certain that only Linux performs runtime patching based on the code seq

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

samitolvanen wrote: > A gentle reminder to please review this PR. Overall this looks fine to me, but I was hoping to get an answer to my question about how the Kconfig detection is actually going to be implemented: https://github.com/llvm/llvm-project/pull/121070#discussion_r1911688455 https:

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

sirmc wrote: I had a quick look (sorry for not getting to this earlier, was traveling around Vietnam for a few weeks). Looks good to me. Especially the trick of encoding the arity into the MOV32ri register seems very neat from a compatibility standpoint. So if I understand this correctly, the

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

scottconstable wrote: @sirmc @samitolvanen @Darksonn @lvwr @maurer @rcvalle A gentle reminder to please review this PR. https://github.com/llvm/llvm-project/pull/121070 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

scottconstable wrote: > > maurer: I think their point is that even if you are not changing the hash > > scheme, you are proposing breaking compatibility of the identifier with > > existing code. Since we don't want to do this many times, if we are > > breaking compatibility with existing code,

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

MaskRay wrote: > maurer: I think their point is that even if you are not changing the hash > scheme, you are proposing breaking compatibility of the identifier with > existing code. Since we don't want to do this many times, if we are breaking > compatibility with existing code, they would lik

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

@@ -254,6 +254,7 @@ FEATURE(is_trivially_constructible, LangOpts.CPlusPlus) FEATURE(is_trivially_copyable, LangOpts.CPlusPlus) FEATURE(is_union, LangOpts.CPlusPlus) FEATURE(kcfi, LangOpts.Sanitize.has(SanitizerKind::KCFI)) +FEATURE(kcfi_x86_arity, LangOpts.Sanitize.has(Sanitize

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

maurer wrote: > > If hashing is changed, consider replacing xxhash64 with xxh3+_64bits > > @MaskRay This PR does not change the hashing scheme at all. I think their point is that even if you are not changing the hash scheme, you are proposing breaking compatibility of the identifier with exist

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

scottconstable wrote: > If hashing is changed, consider replacing xxhash64 with xxh3+_64bits @MaskRay This PR does not change the hashing scheme at all. https://github.com/llvm/llvm-project/pull/121070 ___ cfe-commits mailing list cfe-commits@lists.ll

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

@@ -254,6 +254,7 @@ FEATURE(is_trivially_constructible, LangOpts.CPlusPlus) FEATURE(is_trivially_copyable, LangOpts.CPlusPlus) FEATURE(is_union, LangOpts.CPlusPlus) FEATURE(kcfi, LangOpts.Sanitize.has(SanitizerKind::KCFI)) +FEATURE(kcfi_x86_arity, LangOpts.Sanitize.has(Sanitize

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

MaskRay wrote: If hashing is changed, consider replacing xxhash64 with xxh3+_64bits https://github.com/llvm/llvm-project/pull/121070 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

@@ -254,6 +254,7 @@ FEATURE(is_trivially_constructible, LangOpts.CPlusPlus) FEATURE(is_trivially_copyable, LangOpts.CPlusPlus) FEATURE(is_union, LangOpts.CPlusPlus) FEATURE(kcfi, LangOpts.Sanitize.has(SanitizerKind::KCFI)) +FEATURE(kcfi_x86_arity, LangOpts.Sanitize.has(Sanitize

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

scottconstable wrote: @sirmc @samitolvanen @Darksonn @lvwr @maurer @rcvalle @MaskRay A gentle reminder to please review this PR. https://github.com/llvm/llvm-project/pull/121070 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llv

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

@@ -181,8 +181,26 @@ void X86AsmPrinter::emitKCFITypeId(const MachineFunction &MF) { // Embed the type hash in the X86::MOV32ri instruction to avoid special // casing object file parsers. EmitKCFITypePadding(MF); + + Register MovReg = X86::EAX; + const auto &Triple = M

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

https://github.com/scottconstable updated https://github.com/llvm/llvm-project/pull/121070 >From 73b6f6332cf699b7fa1bcf7cae90ef828cfa9158 Mon Sep 17 00:00:00 2001 From: Scott D Constable Date: Mon, 23 Dec 2024 13:48:48 -0800 Subject: [PATCH] Implement a new kcfi_x86_arity feature that encodes a

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

scottconstable wrote: > And you cannot use ESI when it's used to pass arguments. @phoebewang I think that my https://github.com/llvm/llvm-project/pull/121070#discussion_r1898074522 also applies to this concern. https://github.com/llvm/llvm-project/pull/121070 _

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

@@ -181,8 +181,26 @@ void X86AsmPrinter::emitKCFITypeId(const MachineFunction &MF) { // Embed the type hash in the X86::MOV32ri instruction to avoid special // casing object file parsers. EmitKCFITypePadding(MF); + + Register MovReg = X86::EAX; + const auto &Triple = M

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

@@ -254,6 +254,7 @@ FEATURE(is_trivially_constructible, LangOpts.CPlusPlus) FEATURE(is_trivially_copyable, LangOpts.CPlusPlus) FEATURE(is_union, LangOpts.CPlusPlus) FEATURE(kcfi, LangOpts.Sanitize.has(SanitizerKind::KCFI)) +FEATURE(kcfi_x86_arity, LangOpts.Sanitize.has(Sanitize

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

@@ -181,8 +181,26 @@ void X86AsmPrinter::emitKCFITypeId(const MachineFunction &MF) { // Embed the type hash in the X86::MOV32ri instruction to avoid special // casing object file parsers. EmitKCFITypePadding(MF); + + Register MovReg = X86::EAX; + const auto &Triple = M

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

phoebewang wrote: And you cannot use ESI when it's used to pass arguments. Suggested registers: | Arity Indicator | Description | Encoding in reg field | | --- | --- | --- | | 0 | 0 parameters | EDI | | 1 | 1 parameter in RDI | ESI | | 2 | 2 parameters in RDI a

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

@@ -181,8 +181,26 @@ void X86AsmPrinter::emitKCFITypeId(const MachineFunction &MF) { // Embed the type hash in the X86::MOV32ri instruction to avoid special // casing object file parsers. EmitKCFITypePadding(MF); + + Register MovReg = X86::EAX; + const auto &Triple = M

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

@@ -254,6 +254,7 @@ FEATURE(is_trivially_constructible, LangOpts.CPlusPlus) FEATURE(is_trivially_copyable, LangOpts.CPlusPlus) FEATURE(is_union, LangOpts.CPlusPlus) FEATURE(kcfi, LangOpts.Sanitize.has(SanitizerKind::KCFI)) +FEATURE(kcfi_x86_arity, LangOpts.Sanitize.has(Sanitize

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

@@ -181,8 +181,26 @@ void X86AsmPrinter::emitKCFITypeId(const MachineFunction &MF) { // Embed the type hash in the X86::MOV32ri instruction to avoid special // casing object file parsers. EmitKCFITypePadding(MF); + + Register MovReg = X86::EAX; + const auto &Triple = M

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

https://github.com/scottconstable updated https://github.com/llvm/llvm-project/pull/121070 >From 70f1743e23ff8e218799e94b03a5bf033715666a Mon Sep 17 00:00:00 2001 From: Scott D Constable Date: Mon, 23 Dec 2024 13:48:48 -0800 Subject: [PATCH] Implement a new kcfi_x86_arity feature that encodes a

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

llvmbot wrote: @llvm/pr-subscribers-backend-x86 Author: Scott Constable (scottconstable) Changes Kernel Control Flow Integrity (kCFI) is a feature that hardens indirect calls by comparing a 32-bit hash of the function pointer's type against a hash of the target function's type. If the ha

[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)

https://github.com/scottconstable created https://github.com/llvm/llvm-project/pull/121070 Kernel Control Flow Integrity (kCFI) is a feature that hardens indirect calls by comparing a 32-bit hash of the function pointer's type against a hash of the target function's type. If the hashes do not