Re: [PATCH 00/15] Implement MODVERSIONS for Rust
On Tue, Jul 16, 2024 at 12:12 AM Greg Kroah-Hartman
wrote:
>
> > After replacement:
> >
> > union {
> > u64 new_member;
> > struct {
> > u8 __kabi_reserved_1[8];
> > };
> > }
>
> Note, such a thing would only be for the distros that want it, you can
> add support for this to the tool, but there is no need for any
> __kabi_reserved fields in mainline.
Sure. I assume modversions are primarily useful for distros, so I just
want to make sure this scheme can handle all the common issues as
well.
> > Greg, I know you've been dealing with this for a long time, any thoughts?
>
> It's a good start, yes. Also watch out for when structures go from
> "anonymous" to "fully described" when new #include lines get added to
> files. The current tooling has issues with that, so we need to use
> __GENKSYMS__ #ifdef lines in some places to keep crc generation stable.
> Don't know if dwarf output would be susceptible to the same issues with
> that or not, but you should check.
That's a great point. DWARF output has the exact same problem if a
structure declaration later becomes a definition, but I think we can
come up with a solution for this issue too in v2.
Sami
Re: [PATCH 00/15] Implement MODVERSIONS for Rust
Hi Petr, On Wed, Jul 10, 2024 at 7:30 AM Petr Pavlu wrote: > > On 6/17/24 19:58, Sami Tolvanen wrote: > > Hi folks, > > > > This series implements CONFIG_MODVERSIONS for Rust, an important > > feature for distributions like Android that want to ship Rust > > kernel modules, and depend on modversions to help ensure module ABI > > compatibility. > > Thanks for working on this. Below is some feedback with my (open)SUSE > hat on, although it should be quite general. Great, thanks for taking a look! > > There have been earlier proposals [1][2] that would allow Rust > > modules to coexist with modversions, but none that actually implement > > symbol versioning. Unlike C, Rust source code doesn't have sufficient > > information about the final ABI, as the compiler has considerable > > freedom in adjusting structure layout for improved performance [3], > > for example, which makes using a source code parser like genksyms > > a non-starter. Based on Matt's suggestion and previous feedback > > from maintainers, this series uses DWARF debugging information for > > computing versions. DWARF is an established and relatively stable > > format, which includes all the necessary ABI details, and adding a > > CONFIG_DEBUG_INFO dependency for Rust symbol versioning seems like a > > reasonable trade-off. > > Using the DWARF data makes sense to me. Distribution kernels are > normally built with debuginfo because one has to be able to debug them > later, unsurprisingly. Besides that, one now typically wants to use BPF > together with built-in BTF data (CONFIG_DEBUG_INFO_BTF), which also > requires building the kernel with debuginfo. > > I would however keep in mind that producing all DWARF data has some > cost. From a quick test, an x86_64-defconfig build is ~30% slower for me > with CONFIG_DEBUG_INFO=y. The current genksyms tool allows to have > debuginfo disabled when backporting some patches and consequently have > a quicker feedback whether modversions changed. This option would > disappear with gendwarfksyms but I think it is acceptable. Yes, this matches my benchmarks too. Presumably folks who care about build times and are not interested in debugging information or Rust support would prefer genksyms instead. I'm planning to turn this into a Kconfig choice in the next version. > > The first 12 patches of this series add a small tool for computing > > symbol versions from DWARF, called gendwarfksyms. When passed a list > > of exported symbols, the tool generates an expanded type string > > for each symbol, and computes symbol CRCs similarly to genksyms. > > gendwarfksyms is written in C and uses libdw to process DWARF, mainly > > because of the existing support for C host tools that use elfutils > > (e.g., objtool). > > In addition to calculating CRCs of exported symbols, genksyms has other > features which I think are important. > > Firstly, the genksyms tool has a human-readable storage format for input > data used in the calculation of symbol CRCs. Setting the make variable > KBUILD_SYMTYPES enables dumping this data and storing it in *.symtypes > files. > > When a developer later modifies the kernel and wants to check if some > symbols have changed, they can take these files and feed them as > *.symref back to genksyms. This allows the tool to provide an actual > reason why some symbols have changed, instead of just printing that > their CRCs are different. > > Is there any plan to add the same functionality to gendwarfksyms, or do > you envison that people will use libabigail, Symbol-Type Graph, or > another tool for making this type of comparison? gendwarfksyms also uses human-readable input for the CRC calculations, and it prints out the input strings with the --debug option. I plan to hook this up to KBUILD_SYMTYPES in v2. It should be convenient enough to simply compare the pretty-printed output with diff, so I'm not sure if a built-in comparison option is needed. Any other DWARF analysis tool can be used to spot the differences too, as you mentioned. > Secondly, when distributions want to maintain stable kABI, they need to > be able to deal with patch backports that add new members to structures. > One common approach is to have placeholders in important structures > which can be later replaced by the new members as needed. __GENKSYMS__ > ifdefs are then used at the C source level to hide these kABI-compatible > changes from genksyms. > > Gendwarfksyms works on the resulting binary and so using such ifdefs > wouldn't work. Instead, I suspect that what is required is a mechanism > to tell the tool that a given change is ok, probably by allowing to > specify some map from the original definition to the new one. > &
Re: [PATCH] kallsyms, livepatch: Fix livepatch with CONFIG_LTO_CLANG
On Mon, Jul 8, 2024 at 2:33 PM Luis Chamberlain wrote:
>
> Looking at this again its not to me why Masahiro Yamada's suggestion on
> that old patch series to just increase the length and put long symbols
> names into its own section [0] could not be embraced with a new kconfig
> option, so new kernels and new userspace could support it:
>
> https://lore.kernel.org/lkml/CAK7LNATsuszFR7JB5ZkqVS1W=hWr9=e7btf+mvgj+nxt3az...@mail.gmail.com/
Matt, was there a reason we didn't move forward with Masahiro's
proposal? It sounds reasonable to me, but I might be missing some
background here.
> > I am a bit scared because using hashed symbol names in backtraces, gdb,
> > ... would be a nightmare. Hashes are not human readable and
> > they would complicate the life a lot. And using different names
> > in different interfaces would complicate the life either.
>
> All great points.
>
> The scope of the Rust issue is self contained to modversion_info,
> whereas for CONFIG_LTO_CLANG issue commit 8b8e6b5d3b013b0
> ("kallsyms: strip ThinLTO hashes from static functions") describes
> the issue with userspace tools (it doesn't explain which ones)
> which don't expect the function name to change. This seems to happen
> to static routines so I can only suspect this isn't an issue with
> modversioning as the only symbols that would be used there wouldn't be
> static.
>
> Sami, what was the exact userspace issue with CONFIG_LTO_CLANG and these
> long symbols?
The issue with LTO wasn't symbol length. IIRC the compiler renaming
symbols with ThinLTO caused issues for folks using dynamic kprobes,
and I seem to recall it also breaking systrace in Android, at which
point we decided to strip the postfix in kallsyms to avoid breaking
anything else.
Sami
Re: [PATCH] kallsyms, livepatch: Fix livepatch with CONFIG_LTO_CLANG
Hi Luis, On Fri, Jun 28, 2024 at 10:36 AM Luis Chamberlain wrote: > > On Fri, Jun 28, 2024 at 02:23:49PM +0200, Miroslav Benes wrote: > > On Fri, 7 Jun 2024, Song Liu wrote: > > > > > Hi Miroslav, > > > > > > Thanks for reviewing the patch! > > > > > > I think it is possible. Currently, kallsyms_on_each_match_symbol matches > > > symbols without the postfix. We can add a variation or a parameter, so > > > that it matches the full name with post fix. > > > > I think it might be better. > > > > Luis, what is your take on this? > > > > If I am not mistaken, there was a patch set to address this. Luis might > > remember more. > > Yeah this is a real issue outside of CONFIG_LTO_CLANG, Rust modules is > another example where instead of symbol names they want to use full > hashes. So, as I hinted to you Sami, can we knock two birds with one stone > here and move CONFIG_LTO_CLANG to use the same strategy as Rust so we > have two users instead of just one? I'm all for finding generic solutions, but perhaps I've missed the patch set Miroslav mentioned because I'm not quite sure how these problems are related. LTO makes duplicate symbol names globally unique by appending a postfix to them, which complicates looking up symbols by name. Rust, on the other hand, has a problem with CONFIG_MODVERSIONS because the long symbol names it generates cannot fit in the small buffer in struct modversion_info. The only reason we proposed storing a cryptographic hash in modversion_info was to avoid breaking userspace tools that parse this data structure, but AFAIK nobody wants to use hashed symbol names anywhere else. In fact, if there's a better solution for addressing modversion_info limitations, I would be happy not to hash anything. Sami
Re: [PATCH 00/15] Implement MODVERSIONS for Rust
Hi Luis, On Tue, Jun 18, 2024 at 12:42:51PM -0700, Luis Chamberlain wrote: > On Mon, Jun 17, 2024 at 05:58:19PM +0000, Sami Tolvanen wrote: > > The first 12 patches of this series add a small tool for computing > > symbol versions from DWARF, called gendwarfksyms. When passed a list > > of exported symbols, the tool generates an expanded type string > > for each symbol, and computes symbol CRCs similarly to genksyms. > > So this is too word centric Rust, let's think about this generically. > We still ahve a symbol limitation even in the C world then, and this > solution can solve that problem also for other reasons for *whatever* > reason we devise to-come-up-with-in-the-future for augmenting symbols. > Today Rust, tomorrow, who knows. If you're referring to the symbol hashing in the __versions table, that's not specific to Rust. Rust just happens to be the only source of long symbol names right now. > > gendwarfksyms is written in C and uses libdw to process DWARF, mainly > > because of the existing support for C host tools that use elfutils > > (e.g., objtool). > > I agree with Masahiro, that testing this with vmlinux would be eye > opening to what challenges we really have ahead. So, to help with this > let's try to re-think about this from another perspective. > > Yes, old userspace should not break, but you can add yet another option > to let you opt-in to a new world order of how these crc are mapped to > hashed repersentations of the symbols. This would allow us to: We did run into an issue with depmod in our testing, where it needs to be taught about hashed names to avoid 'unknown symbol' warnings. I'm not sure if this is acceptable, so I would like to hear feedback about the viability of the hashing scheme in general. If old userspace can't have any regressions because of long symbol names, I suspect we'll have to go back to omitting long symbols from struct modversion_info and adding a v2 of the __versions table with no symbol name length limitations. Happy to hear your thoughts. > a) Ensure correctness for all users / tools, so that proper plumbing is >really done. By considering all symbols you increase your scope of >awareness of anything that could really break. > > b) Remove the "Rust" nature about this > > c) Rust modules just becomes a *user* of this approach I believe the only Rust nature here is the last patch that enables gendwarfksyms only for Rust. Otherwise, there shouldn't be anything strictly Rust specific. > It gets me wondering, given Kris is also working on allowing traces to > map symbols to module names, how does this fit into that world [0]? AFAIK long symbol names are only a problem for struct modversion_info, which uses a relatively short name buffer, so I'm hoping other efforts won't be affected. > As for a) the reason I'm thinking about having the ability to test a > full real kernel and moules with this is, without that, how are you sure > you have the full scope of the changes needed? Sure, I can look into hooking this up for the entire kernel and seeing what breaks, in addition the issues Masahiro pointed out, of course. Sami
Re: [PATCH 00/15] Implement MODVERSIONS for Rust
On Wed, Jun 19, 2024 at 04:03:45AM +0900, Masahiro Yamada wrote: > On Wed, Jun 19, 2024 at 2:18 AM Greg Kroah-Hartman > wrote: > > > > On Wed, Jun 19, 2024 at 01:50:36AM +0900, Masahiro Yamada wrote: > > > On Wed, Jun 19, 2024 at 1:44 AM Greg Kroah-Hartman > > > wrote: > > > > > > > > That's cool, can the C code be switched to also use this? That way we > > > > only have one path/code for all of this? > > > > > > > > > As the description says, it requires CONFIG_DEBUG_INFO. > > > We can strip the debug info from the final vmlinux, but > > > I guess the build speed will be even slower than the current genksyms. > > > > For people who want genksyms (i.e. distros), don't they normally already > > enable DEBUG_INFO as well? The problems of genksyms are well known and > > a pain (I speak from experience), so replacing it with info based on > > DWARF would be great, I'll gladly trade off the DEBUG_INFO issue for > > stablilty! > > > > thanks, > > > > greg k-h > > > > > > I do not think gendwarfksyms is a drop-in replacement, > because it relies on libelf and libdw, which will not > work with LLVM bitcode when CONFIG_LTO_CLANG=y. > > His "Let's postpone this until final linking" stuff will > come back? > Then, vmlinux.o is processed to extract the CRC > of all symbols? I agree, this won't work with LTO unless we process vmlinux.o. > In my benchmark, this tool took 3.84 sec just for processing > a single rust/core.o object. To be fair, Rust currently exports all globals and core.o has 400 exported symbols as a result. During my brief testing, this tool is faster than genksyms for normal C code. > I'd love to see how long it will take to process vmlinux.o It's obviously going to be quite slow, my defconfig vmlinux.o has 14k exported symbols: Performance counter stats for './tools/gendwarfksyms/gendwarfksyms vmlinux.o': 371,527.67 msec task-clock:u #1.000 CPUs utilized 0 context-switches:u #0.000 /sec 0 cpu-migrations:u #0.000 /sec 231,554 page-faults:u# 623.248 /sec cycles:u instructions:u branches:u branch-misses:u 371.686151684 seconds time elapsed 370.534637000 seconds user 0.987825000 seconds sys The tool is currently single-threaded, so if we really want to go this route, it could probably be made a bit faster. > And this occurs even when a single source file is changed > and vmlinux.o is re-linked. I suppose anyone using LTO already knows it won't be a quick rebuild though. Sami
Re: [PATCH 13/15] modpost: Add support for hashing long symbol names
On Wed, Jun 19, 2024 at 01:47:13AM +0900, Masahiro Yamada wrote:
> On Tue, Jun 18, 2024 at 2:58 AM Sami Tolvanen wrote:
> >
> > +#define barrier_data(ptr) __asm__ __volatile__("": :"r"(ptr) :"memory")
>
>
>
>
> I know this is a copy-paste of the kernel space code,
> but is barrier_data() also necessary for host programs?
I tested this with Clang and it worked fine without the barrier, but
with gcc, the code no longer produces correct values. Presumably this is
a load bearing data barrier.
Sami
Re: [PATCH 00/15] Implement MODVERSIONS for Rust
Hi Masahiro, On Wed, Jun 19, 2024 at 01:28:21AM +0900, Masahiro Yamada wrote: > I am surprised at someone who attempts to add another variant of genksyms. The options are rather limited if we want Rust modules that are compatible with modversions. We either come up with a way to version Rust symbols or we bypass version checks for them, which is basically what Matt's earlier patch set did: https://lore.kernel.org/rust-for-linux/20231118025748.2778044-1-mmau...@google.com/ If there are better solutions, I would be happy to hear them. > I am also surprised at the tool being added under the tools/ directory. I picked this location because that's where objtool lives, and wasn't aware that this was frowned upon. I'm fine with moving the tool elsewhere too. Sami
[PATCH 10/15] gendwarfksyms: Expand structure types
Recursively expand DWARF structure types, e.g. structs, unions, and
enums. Type strings also encode structure member layout, which allows
us to determine ABI breakages if the compiler decides to reorder
members or otherwise change the layout.
Example output with --debug:
subprogram(
formal_parameter pointer_type *mut {
structure_type {
member pointer_type {
base_type u8 byte_size(1)
} data_member_location(0),
member base_type usize byte_size(8) data_member_location(8),
} byte_size(16) alignment(8)
},
)
-> base_type void;
Signed-off-by: Sami Tolvanen
---
tools/gendwarfksyms/gendwarfksyms.h | 5 ++
tools/gendwarfksyms/types.c | 127 +++-
2 files changed, 130 insertions(+), 2 deletions(-)
diff --git a/tools/gendwarfksyms/gendwarfksyms.h
b/tools/gendwarfksyms/gendwarfksyms.h
index 03d8a4a039c3..4646eaf5c85e 100644
--- a/tools/gendwarfksyms/gendwarfksyms.h
+++ b/tools/gendwarfksyms/gendwarfksyms.h
@@ -52,8 +52,13 @@ extern bool no_pretty_print;
})
/* Consistent aliases (DW_TAG__type) for DWARF tags */
+#define DW_TAG_enumerator_type DW_TAG_enumerator
#define DW_TAG_formal_parameter_type DW_TAG_formal_parameter
+#define DW_TAG_member_type DW_TAG_member
+#define DW_TAG_template_type_parameter_type DW_TAG_template_type_parameter
#define DW_TAG_typedef_type DW_TAG_typedef
+#define DW_TAG_variant_part_type DW_TAG_variant_part
+#define DW_TAG_variant_type DW_TAG_variant
/*
* symbols.c
diff --git a/tools/gendwarfksyms/types.c b/tools/gendwarfksyms/types.c
index b1b82d166eb8..fa74e6fc26e3 100644
--- a/tools/gendwarfksyms/types.c
+++ b/tools/gendwarfksyms/types.c
@@ -166,8 +166,10 @@ static int process_fqn(struct state *state, struct
cached_die *cache,
return 0; \
}
+DEFINE_PROCESS_UDATA_ATTRIBUTE(accessibility)
DEFINE_PROCESS_UDATA_ATTRIBUTE(alignment)
DEFINE_PROCESS_UDATA_ATTRIBUTE(byte_size)
+DEFINE_PROCESS_UDATA_ATTRIBUTE(data_member_location)
/* Match functions -- die_match_callback_t */
#define DEFINE_MATCH(type) \
@@ -176,8 +178,11 @@ DEFINE_PROCESS_UDATA_ATTRIBUTE(byte_size)
return dwarf_tag(die) == DW_TAG_##type##_type; \
}
+DEFINE_MATCH(enumerator)
DEFINE_MATCH(formal_parameter)
+DEFINE_MATCH(member)
DEFINE_MATCH(subrange)
+DEFINE_MATCH(variant)
bool match_all(Dwarf_Die *die)
{
@@ -222,6 +227,8 @@ static int __process_list_type(struct state *state, struct
cached_die *cache,
{
check(process(state, cache, type));
check(process_type_attr(state, cache, die));
+ check(process_accessibility_attr(state, cache, die));
+ check(process_data_member_location_attr(state, cache, die));
check(process(state, cache, ","));
return check(process_linebreak(cache, 0));
}
@@ -234,6 +241,7 @@ static int __process_list_type(struct state *state, struct
cached_die *cache,
}
DEFINE_PROCESS_LIST_TYPE(formal_parameter)
+DEFINE_PROCESS_LIST_TYPE(member)
/* Container types with DW_AT_type */
static int __process_type(struct state *state, struct cached_die *cache,
@@ -266,6 +274,7 @@ DEFINE_PROCESS_TYPE(reference)
DEFINE_PROCESS_TYPE(restrict)
DEFINE_PROCESS_TYPE(rvalue_reference)
DEFINE_PROCESS_TYPE(shared)
+DEFINE_PROCESS_TYPE(template_type_parameter)
DEFINE_PROCESS_TYPE(volatile)
DEFINE_PROCESS_TYPE(typedef)
@@ -318,6 +327,110 @@ static int process_subroutine_type(struct state *state,
return check(__process_subroutine_type(state, cache, die,
"subroutine_type"));
}
+
+static int process_variant_type(struct state *state, struct cached_die *cache,
+ Dwarf_Die *die)
+{
+ return check(process_die_container(state, cache, die, process_type,
+ match_member_type));
+}
+
+static int process_variant_part_type(struct state *state,
+struct cached_die *cache, Dwarf_Die *die)
+{
+ check(process(state, cache, "variant_part {"));
+ check(process_linebreak(cache, 1));
+ check(process_die_container(state, cache, die, process_type,
+ match_variant_type));
+ check(process_linebreak(cache, -1));
+ check(process(state, cache, "},"));
+ return check(process_linebreak(cache, 0));
+}
+
+static int ___process_structure_type(struct state *state,
+struct cached_die *cache, Dwarf_Die *die)
+{
+ switch (dwarf_tag(die)) {
+ case DW_TAG_member:
+ case DW_TAG_variant_part:
+ return check(process_type(state, cache, die));
+ case DW_TAG_class_type:
+ case DW_TAG_enumeration_type:
+ case DW_TAG_structure_type:
+ case DW_TAG_template_type_parameter:
+ ca
[PATCH 15/15] kbuild: Use gendwarfksyms to generate Rust symbol versions
Use gendwarfksyms to generate symbol versions for exported Rust symbols,
and allow CONFIG_MODVERSIONS to be enabled with CONFIG_RUST, assuming the
debugging information needed by gendwarfksyms is also available.
Signed-off-by: Sami Tolvanen
---
Makefile | 6 ++
init/Kconfig | 2 +-
rust/Makefile | 30 --
3 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/Makefile b/Makefile
index 14427547dc1e..0701917c5172 100644
--- a/Makefile
+++ b/Makefile
@@ -1344,6 +1344,12 @@ prepare: tools/bpf/resolve_btfids
endif
endif
+ifdef CONFIG_MODVERSIONS
+ifdef CONFIG_RUST
+prepare: tools/gendwarfksyms
+endif
+endif
+
PHONY += resolve_btfids_clean
resolve_btfids_O = $(abspath $(objtree))/tools/bpf/resolve_btfids
diff --git a/init/Kconfig b/init/Kconfig
index 72404c1f2157..ef29f002e932 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1903,7 +1903,7 @@ config RUST
depends on HAVE_RUST
depends on RUST_IS_AVAILABLE
depends on !CFI_CLANG
- depends on !MODVERSIONS
+ depends on !MODVERSIONS || (DEBUG_INFO && !DEBUG_INFO_REDUCED &&
!DEBUG_INFO_SPLIT)
depends on !GCC_PLUGINS
depends on !RANDSTRUCT
depends on !DEBUG_INFO_BTF || PAHOLE_HAS_LANG_EXCLUDE
diff --git a/rust/Makefile b/rust/Makefile
index f70d5e244fee..385cdf5dc320 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -356,10 +356,11 @@ $(obj)/bindings/bindings_helpers_generated.rs: private
bindgen_target_extra = ;
$(obj)/bindings/bindings_helpers_generated.rs: $(src)/helpers.c FORCE
$(call if_changed_dep,bindgen)
+rust_exports = $(NM) -p --defined-only $(1) | awk '/ (T|R|D) / { printf
$(2),$(3) }'
+
quiet_cmd_exports = EXPORTS $@
cmd_exports = \
- $(NM) -p --defined-only $< \
- | awk '/ (T|R|D) / {printf
"EXPORT_SYMBOL_RUST_GPL(%s);\n",$$3}' > $@
+ $(call rust_exports,$<,"EXPORT_SYMBOL_RUST_GPL(%s);\n",$$3) > $@
$(obj)/exports_core_generated.h: $(obj)/core.o FORCE
$(call if_changed,exports)
@@ -420,12 +421,29 @@ ifneq ($(or $(CONFIG_ARM64),$(and
$(CONFIG_RISCV),$(CONFIG_64BIT))),)
__ashlti3 __lshrti3
endif
+ifdef CONFIG_MODVERSIONS
+# When module versioning is enabled, calculate symbol versions from the DWARF
+# debugging information. We can't use a source code parser like genksyms,
+# because the source files don't have information about the final structure
+# layout and emitted symbols.
+gendwarfksyms := $(objtree)/tools/gendwarfksyms/gendwarfksyms
+
+cmd_gendwarfksyms = \
+ $(call rust_exports,$@,"%s %s\n",$$1$(comma)$$3) \
+ | $(gendwarfksyms) $@ >> $(dot-target).cmd
+endif
+
+define rule_rustc_library_with_exports
+ $(call cmd_and_fixdep,rustc_library)
+ $(call cmd,gendwarfksyms)
+endef
+
$(obj)/core.o: private skip_clippy = 1
$(obj)/core.o: private skip_flags = -Dunreachable_pub
$(obj)/core.o: private rustc_objcopy = $(foreach
sym,$(redirect-intrinsics),--redefine-sym $(sym)=__rust$(sym))
$(obj)/core.o: private rustc_target_flags = $(core-cfgs)
$(obj)/core.o: $(RUST_LIB_SRC)/core/src/lib.rs FORCE
- +$(call if_changed_dep,rustc_library)
+ +$(call if_changed_rule,rustc_library_with_exports)
ifdef CONFIG_X86_64
$(obj)/core.o: scripts/target.json
endif
@@ -438,7 +456,7 @@ $(obj)/alloc.o: private skip_clippy = 1
$(obj)/alloc.o: private skip_flags = -Dunreachable_pub
$(obj)/alloc.o: private rustc_target_flags = $(alloc-cfgs)
$(obj)/alloc.o: $(RUST_LIB_SRC)/alloc/src/lib.rs $(obj)/compiler_builtins.o
FORCE
- +$(call if_changed_dep,rustc_library)
+ +$(call if_changed_rule,rustc_library_with_exports)
$(obj)/build_error.o: $(src)/build_error.rs $(obj)/compiler_builtins.o FORCE
+$(call if_changed_dep,rustc_library)
@@ -447,7 +465,7 @@ $(obj)/bindings.o: $(src)/bindings/lib.rs \
$(obj)/compiler_builtins.o \
$(obj)/bindings/bindings_generated.rs \
$(obj)/bindings/bindings_helpers_generated.rs FORCE
- +$(call if_changed_dep,rustc_library)
+ +$(call if_changed_rule,rustc_library_with_exports)
$(obj)/uapi.o: $(src)/uapi/lib.rs \
$(obj)/compiler_builtins.o \
@@ -458,6 +476,6 @@ $(obj)/kernel.o: private rustc_target_flags = --extern
alloc \
--extern build_error --extern macros --extern bindings --extern uapi
$(obj)/kernel.o: $(src)/kernel/lib.rs $(obj)/alloc.o $(obj)/build_error.o \
$(obj)/libmacros.so $(obj)/bindings.o $(obj)/uapi.o FORCE
- +$(call if_changed_dep,rustc_library)
+ +$(call if_changed_rule,rustc_library_with_exports)
endif # CONFIG_RUST
--
2.45.2.627.g7a2c4fd464-goog
[PATCH 14/15] module: Support hashed symbol names when checking modversions
When checking modversions for symbol names longer than MODULE_NAME_LEN,
look for the hashed symbol name instead. This is needed for Rust modules,
which frequently use symbol names longer than the maximum length
supported by struct modversion_info.
Suggested-by: Matthew Maurer
Signed-off-by: Sami Tolvanen
---
kernel/module/version.c | 38 +-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/kernel/module/version.c b/kernel/module/version.c
index 53f43ac5a73e..0b33ca085138 100644
--- a/kernel/module/version.c
+++ b/kernel/module/version.c
@@ -8,8 +8,36 @@
#include
#include
#include
+#include
#include "internal.h"
+/*
+ * For symbol names longer than MODULE_NAME_LEN, the version table includes
+ * a hash of the symbol name in the following format:
+ *
+ * \0
+ */
+#define SYMHASH_PREFIX "sha256"
+#define SYMHASH_PREFIX_LEN sizeof(SYMHASH_PREFIX) /* includes \0 */
+#define SYMHASH_LEN(SYMHASH_PREFIX_LEN + SHA256_DIGEST_SIZE)
+
+static void symhash(const char *name, size_t len, u8 hash[SYMHASH_LEN])
+{
+ memcpy(hash, SYMHASH_PREFIX, SYMHASH_PREFIX_LEN);
+ sha256(name, len, [SYMHASH_PREFIX_LEN]);
+}
+
+static int symcmp(const char *version_name, const char *name, size_t len,
+ const u8 *hash)
+{
+ BUILD_BUG_ON(SYMHASH_LEN > MODULE_NAME_LEN);
+
+ if (len >= MODULE_NAME_LEN)
+ return memcmp(version_name, hash, SYMHASH_LEN);
+
+ return strcmp(version_name, name);
+}
+
int check_version(const struct load_info *info,
const char *symname,
struct module *mod,
@@ -19,6 +47,8 @@ int check_version(const struct load_info *info,
unsigned int versindex = info->index.vers;
unsigned int i, num_versions;
struct modversion_info *versions;
+ u8 hash[SYMHASH_LEN];
+ size_t len;
/* Exporting module didn't supply crcs? OK, we're already tainted. */
if (!crc)
@@ -32,10 +62,16 @@ int check_version(const struct load_info *info,
num_versions = sechdrs[versindex].sh_size
/ sizeof(struct modversion_info);
+ len = strlen(symname);
+
+ /* For symbols with a long name, use the hash format. */
+ if (len >= MODULE_NAME_LEN)
+ symhash(symname, len, hash);
+
for (i = 0; i < num_versions; i++) {
u32 crcval;
- if (strcmp(versions[i].name, symname) != 0)
+ if (symcmp(versions[i].name, symname, len, hash) != 0)
continue;
crcval = *crc;
--
2.45.2.627.g7a2c4fd464-goog
[PATCH 13/15] modpost: Add support for hashing long symbol names
Rust frequently has symbol names longer than MODULE_NAME_LEN, because
the full namespace path is encoded into the mangled name. Instead of
modpost failing when running into a long name with CONFIG_MODVERSIONS,
store a hash of the name in struct modversion_info.
To avoid breaking userspace tools that parse the __versions array,
include a null-terminated hash function name at the beginning of the
name string, followed by a binary hash. In order to keep .mod.c files
more human-readable, add a comment with the full symbol name before the
array entry. Example output in rust_minimal.mod.c:
static const struct modversion_info versions[]
__used __section("__versions") = {
/* _RNvNtNtCs1cdwasc6FUb_6kernel5print14format_strings4INFO */
{ 0x9ec5442f, "sha256\x00\x56\x96\xf4\x27\xdb\x4a\xbf[...]" },
{ 0x1d6595b1, "_RNvNtCs1cdwasc6FUb_6kernel5print11call_printk" },
{ 0x3c642974, "__rust_dealloc" },
...
};
modprobe output for the resulting module:
$ modprobe --dump-modversions rust_minimal.ko
0x9ec5442fsha256
0x1d6595b1_RNvNtCs1cdwasc6FUb_6kernel5print11call_printk
0x3c642974__rust_dealloc
...
While the output is less useful, the tool continues to work and can be
later updated to produce more helpful output for hashed symbols.
Note that this patch adds a generic SHA-256 implementation to modpost
adapted from the Crypto API, but other hash functions may be used in
future if needed.
Suggested-by: Matthew Maurer
Signed-off-by: Sami Tolvanen
---
scripts/mod/Makefile | 4 +-
scripts/mod/modpost.c | 20 ++-
scripts/mod/modpost.h | 20 +++
scripts/mod/symhash.c | 327 ++
4 files changed, 364 insertions(+), 7 deletions(-)
create mode 100644 scripts/mod/symhash.c
diff --git a/scripts/mod/Makefile b/scripts/mod/Makefile
index c729bc936bae..ddd59ea9027e 100644
--- a/scripts/mod/Makefile
+++ b/scripts/mod/Makefile
@@ -4,7 +4,7 @@ CFLAGS_REMOVE_empty.o += $(CC_FLAGS_LTO)
hostprogs-always-y += modpost mk_elfconfig
always-y += empty.o
-modpost-objs := modpost.o file2alias.o sumversion.o symsearch.o
+modpost-objs := modpost.o file2alias.o symhash.o sumversion.o symsearch.o
devicetable-offsets-file := devicetable-offsets.h
@@ -15,7 +15,7 @@ targets += $(devicetable-offsets-file) devicetable-offsets.s
# dependencies on generated files need to be listed explicitly
-$(obj)/modpost.o $(obj)/file2alias.o $(obj)/sumversion.o $(obj)/symsearch.o:
$(obj)/elfconfig.h
+$(obj)/modpost.o $(obj)/file2alias.o $(obj)/symhash.o $(obj)/sumversion.o
$(obj)/symsearch.o: $(obj)/elfconfig.h
$(obj)/file2alias.o: $(obj)/$(devicetable-offsets-file)
quiet_cmd_elfconfig = MKELF $@
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index f48d72d22dc2..2631e3e75a5c 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1900,7 +1900,10 @@ static void add_exported_symbols(struct buffer *buf,
struct module *mod)
**/
static void add_versions(struct buffer *b, struct module *mod)
{
+ char hash[SYMHASH_STR_LEN];
struct symbol *s;
+ const char *name;
+ size_t len;
if (!modversions)
return;
@@ -1917,13 +1920,20 @@ static void add_versions(struct buffer *b, struct
module *mod)
s->name, mod->name);
continue;
}
- if (strlen(s->name) >= MODULE_NAME_LEN) {
- error("too long symbol \"%s\" [%s.ko]\n",
- s->name, mod->name);
- break;
+ len = strlen(s->name);
+ /*
+* For symbols with a long name, use the hash format, but
include
+* the full symbol name as a comment to keep the generated files
+* human-readable.
+*/
+ if (len >= MODULE_NAME_LEN) {
+ buf_printf(b, "\t/* %s */\n", s->name);
+ name = symhash_str(s->name, len, hash);
+ } else {
+ name = s->name;
}
buf_printf(b, "\t{ %#8x, \"%s\" },\n",
- s->crc, s->name);
+ s->crc, name);
}
buf_printf(b, "};\n");
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index ee43c7950636..cd2eb718936b 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -183,6 +183,26 @@ void handle_moddevtable(struct module *mod, struct
elf_info *info,
Elf_Sym *sym, const char *symname);
void add_moddevtable(struct buffer *buf, struct module *mod);
+/* symhash.c */
+/*
+ * For symbol names longer than MODULE_NAME_LEN, encode a hash of the
+ * symbol name in version information as fo
[PATCH 12/15] gendwarfksyms: Add inline debugging
Understanding the operation and caching of the tool can be somewhat
challenging without a debugger. Add inline debugging information with
the --inline-debug command, which adds highlighted tags to the --debug
output with information about cache states etc.
Signed-off-by: Sami Tolvanen
---
tools/gendwarfksyms/gendwarfksyms.c | 3 +++
tools/gendwarfksyms/gendwarfksyms.h | 29 +
tools/gendwarfksyms/types.c | 14 ++
3 files changed, 46 insertions(+)
diff --git a/tools/gendwarfksyms/gendwarfksyms.c
b/tools/gendwarfksyms/gendwarfksyms.c
index 7095f0ecccab..acf4b44238e2 100644
--- a/tools/gendwarfksyms/gendwarfksyms.c
+++ b/tools/gendwarfksyms/gendwarfksyms.c
@@ -15,6 +15,8 @@
/* Print type descriptions and debugging output to stderr */
bool debug;
+/* Print inline debugging information to stderr (with --debug) */
+bool inline_debug;
/* Don't use caching */
bool no_cache;
/* Don't pretty-print (with --debug) */
@@ -25,6 +27,7 @@ static const struct {
bool *flag;
} options[] = {
{ "--debug", },
+ { "--inline-debug", _debug },
{ "--no-cache", _cache },
{ "--no-pretty-print", _pretty_print },
};
diff --git a/tools/gendwarfksyms/gendwarfksyms.h
b/tools/gendwarfksyms/gendwarfksyms.h
index 568f3727017e..34d1be3cfb7f 100644
--- a/tools/gendwarfksyms/gendwarfksyms.h
+++ b/tools/gendwarfksyms/gendwarfksyms.h
@@ -18,6 +18,7 @@
* Options -- in gendwarfksyms.c
*/
extern bool debug;
+extern bool inline_debug;
extern bool no_cache;
extern bool no_pretty_print;
@@ -38,6 +39,18 @@ extern bool no_pretty_print;
#define warn(format, ...) __println("warning: ", format, ##__VA_ARGS__)
#define error(format, ...) __println("error: ", format, ##__VA_ARGS__)
+#define __inline_debug(color, format, ...) \
+ do {\
+ if (debug && inline_debug) \
+ fprintf(stderr, \
+ "\033[" #color "m<" format ">\033[39m", \
+ __VA_ARGS__); \
+ } while (0)
+
+#define inline_debug_r(format, ...) __inline_debug(91, format, __VA_ARGS__)
+#define inline_debug_g(format, ...) __inline_debug(92, format, __VA_ARGS__)
+#define inline_debug_b(format, ...) __inline_debug(94, format, __VA_ARGS__)
+
/*
* Error handling helpers
*/
@@ -51,6 +64,9 @@ extern bool no_pretty_print;
__res; \
})
+/*
+ * DWARF helpers
+ */
/* Consistent aliases (DW_TAG__type) for DWARF tags */
#define DW_TAG_enumerator_type DW_TAG_enumerator
#define DW_TAG_formal_parameter_type DW_TAG_formal_parameter
@@ -97,6 +113,19 @@ struct cached_item {
enum cached_die_state { INCOMPLETE, UNEXPANDED, COMPLETE };
+static inline const char *cache_state_name(enum cached_die_state state)
+{
+ switch (state) {
+ default:
+ case INCOMPLETE:
+ return "INCOMPLETE";
+ case UNEXPANDED:
+ return "UNEXPANDED";
+ case COMPLETE:
+ return "COMPLETE";
+ }
+}
+
struct cached_die {
enum cached_die_state state;
uintptr_t addr;
diff --git a/tools/gendwarfksyms/types.c b/tools/gendwarfksyms/types.c
index da3aa2ad9f57..694b33cdd606 100644
--- a/tools/gendwarfksyms/types.c
+++ b/tools/gendwarfksyms/types.c
@@ -97,6 +97,7 @@ static int process(struct state *state, struct cached_die
*cache, const char *s)
fputs(s, stderr);
state->crc = partial_crc32(s, state->crc);
+ inline_debug_r("cache %p string '%s'", cache, s);
return cache_add_string(cache, s);
}
@@ -456,6 +457,8 @@ static int process_cached(struct state *state, struct
cached_die *cache,
while (ci) {
switch (ci->type) {
case STRING:
+ inline_debug_b("cache %p STRING '%s'", cache,
+ ci->data.str);
check(process(state, NULL, ci->data.str));
break;
case LINEBREAK:
@@ -468,6 +471,8 @@ static int process_cached(struct state *state, struct
cached_die *cache,
error("dwarf_die_addr_die failed");
return -1;
}
+ inline_debug_b("cache %p DIE addr %lx tag %d", cache,
+ ci->data.addr, dwarf_tag());
check(process_type(state, NULL, ));
break;
default:
@@ -561,6 +566,9 @@ static int process_type(struct state *state, struct
[PATCH 11/15] gendwarfksyms: Limit structure expansion
Expand each structure type only once per exported symbol. This
is necessary to support self-referential structures, which would
otherwise result in infinite recursion. Expanding each structure type
just once is enough to catch ABI changes.
For pointers to structure types, limit expansion to three levels
inside the pointer. This should be plenty for catching ABI differences
and stops us from pulling in half the kernel for structs that contain
pointers to large structs like task_struct.
Signed-off-by: Sami Tolvanen
---
tools/gendwarfksyms/cache.c | 49
tools/gendwarfksyms/gendwarfksyms.h | 9 ++-
tools/gendwarfksyms/types.c | 87 ++---
3 files changed, 136 insertions(+), 9 deletions(-)
diff --git a/tools/gendwarfksyms/cache.c b/tools/gendwarfksyms/cache.c
index 85a2adeb649d..1d6eb27d799d 100644
--- a/tools/gendwarfksyms/cache.c
+++ b/tools/gendwarfksyms/cache.c
@@ -158,3 +158,52 @@ int cache_add_die(struct cached_die *cd, Dwarf_Die *die)
ci->type = DIE;
return 0;
}
+
+/* A list of structure types that were already expanded for the current symbol
*/
+struct expanded {
+ uintptr_t addr;
+ struct hlist_node hash;
+};
+
+/* die->addr -> struct expanded */
+DEFINE_HASHTABLE(expansion_cache, DIE_HASH_BITS);
+
+int cache_mark_expanded(Dwarf_Die *die)
+{
+ struct expanded *es;
+
+ es = malloc(sizeof(struct expanded));
+ if (!es) {
+ error("malloc failed");
+ return -1;
+ }
+
+ es->addr = (uintptr_t)die->addr;
+ hash_add(expansion_cache, >hash, es->addr);
+ return 0;
+}
+
+bool cache_was_expanded(Dwarf_Die *die)
+{
+ struct expanded *es;
+ uintptr_t addr = (uintptr_t)die->addr;
+
+ hash_for_each_possible(expansion_cache, es, hash, addr) {
+ if (es->addr == addr)
+ return true;
+ }
+
+ return false;
+}
+
+void cache_clear_expanded(void)
+{
+ struct hlist_node *tmp;
+ struct expanded *es;
+ int i;
+
+ hash_for_each_safe(expansion_cache, i, tmp, es, hash) {
+ free(es);
+ }
+ hash_init(expansion_cache);
+}
diff --git a/tools/gendwarfksyms/gendwarfksyms.h
b/tools/gendwarfksyms/gendwarfksyms.h
index 4646eaf5c85e..568f3727017e 100644
--- a/tools/gendwarfksyms/gendwarfksyms.h
+++ b/tools/gendwarfksyms/gendwarfksyms.h
@@ -95,7 +95,7 @@ struct cached_item {
struct cached_item *next;
};
-enum cached_die_state { INCOMPLETE, COMPLETE };
+enum cached_die_state { INCOMPLETE, UNEXPANDED, COMPLETE };
struct cached_die {
enum cached_die_state state;
@@ -111,6 +111,10 @@ extern int cache_add_linebreak(struct cached_die *pd, int
linebreak);
extern int cache_add_die(struct cached_die *pd, Dwarf_Die *die);
extern void cache_free(void);
+extern int cache_mark_expanded(Dwarf_Die *die);
+extern bool cache_was_expanded(Dwarf_Die *die);
+extern void cache_clear_expanded(void);
+
/*
* types.c
*/
@@ -120,6 +124,9 @@ struct state {
Dwarf *dbg;
struct symbol *sym;
Dwarf_Die origin;
+ unsigned int ptr_expansion_depth;
+ bool in_pointer_type;
+ bool expand;
unsigned long crc;
};
diff --git a/tools/gendwarfksyms/types.c b/tools/gendwarfksyms/types.c
index fa74e6fc26e3..da3aa2ad9f57 100644
--- a/tools/gendwarfksyms/types.c
+++ b/tools/gendwarfksyms/types.c
@@ -381,14 +381,21 @@ static int __process_structure_type(struct state *state,
check(process(state, cache, " {"));
check(process_linebreak(cache, 1));
- check(process_die_container(state, cache, die, process_func,
- match_func));
+ if (state->expand) {
+ check(cache_mark_expanded(die));
+ check(process_die_container(state, cache, die, process_func,
+ match_func));
+ } else {
+ check(process(state, cache, ""));
+ }
check(process_linebreak(cache, -1));
check(process(state, cache, "}"));
- check(process_byte_size_attr(state, cache, die));
- check(process_alignment_attr(state, cache, die));
+ if (state->expand) {
+ check(process_byte_size_attr(state, cache, die));
+ check(process_alignment_attr(state, cache, die));
+ }
return 0;
}
@@ -475,9 +482,38 @@ static int process_cached(struct state *state, struct
cached_die *cache,
static void state_init(struct state *state)
{
+ state->ptr_expansion_depth = 0;
+ state->in_pointer_type = false;
+ state->expand = true;
state->crc = 0x;
}
+static void state_restore(struct state *state, struct state *saved)
+{
+ state->ptr_expansion_depth = saved->ptr_expansion_depth;
+ state->in_pointer_type = saved->in_pointer_type
[PATCH 09/15] gendwarfksyms: Expand array_type
Add support for expanding DW_TAG_array_type, and the subrange type
indicating array size.
Example output with --debug:
variable array_type [34] {
pointer_type {
const_type {
base_type char byte_size(1)
}
}
};
Signed-off-by: Sami Tolvanen
---
tools/gendwarfksyms/types.c | 28
1 file changed, 28 insertions(+)
diff --git a/tools/gendwarfksyms/types.c b/tools/gendwarfksyms/types.c
index a56aeaa4f3a1..b1b82d166eb8 100644
--- a/tools/gendwarfksyms/types.c
+++ b/tools/gendwarfksyms/types.c
@@ -177,6 +177,7 @@ DEFINE_PROCESS_UDATA_ATTRIBUTE(byte_size)
}
DEFINE_MATCH(formal_parameter)
+DEFINE_MATCH(subrange)
bool match_all(Dwarf_Die *die)
{
@@ -268,6 +269,31 @@ DEFINE_PROCESS_TYPE(shared)
DEFINE_PROCESS_TYPE(volatile)
DEFINE_PROCESS_TYPE(typedef)
+static int process_subrange_type(struct state *state, struct cached_die *cache,
+Dwarf_Die *die)
+{
+ Dwarf_Word count = 0;
+
+ if (get_udata_attr(die, DW_AT_count, ))
+ return check(process_fmt(state, cache, "[%" PRIu64 "]", count));
+
+ return check(process(state, cache, "[]"));
+}
+
+static int process_array_type(struct state *state, struct cached_die *cache,
+ Dwarf_Die *die)
+{
+ check(process(state, cache, "array_type "));
+ /* Array size */
+ check(process_die_container(state, cache, die, process_type,
+ match_subrange_type));
+ check(process(state, cache, " {"));
+ check(process_linebreak(cache, 1));
+ check(process_type_attr(state, cache, die));
+ check(process_linebreak(cache, -1));
+ return check(process(state, cache, "}"));
+}
+
static int __process_subroutine_type(struct state *state,
struct cached_die *cache, Dwarf_Die *die,
const char *type)
@@ -378,7 +404,9 @@ static int process_type(struct state *state, struct
cached_die *parent,
PROCESS_TYPE(volatile)
/* Subtypes */
PROCESS_TYPE(formal_parameter)
+ PROCESS_TYPE(subrange)
/* Other types */
+ PROCESS_TYPE(array)
PROCESS_TYPE(base)
PROCESS_TYPE(subroutine)
PROCESS_TYPE(typedef)
--
2.45.2.627.g7a2c4fd464-goog
[PATCH 08/15] gendwarfksyms: Expand subroutine_type
Add support for expanding DW_TAG_subroutine_type and the parameters
in DW_TAG_formal_parameter. Use also to expand subprograms.
Example output with --debug:
subprogram(
formal_parameter base_type usize byte_size(8),
formal_parameter base_type usize byte_size(8),
)
-> base_type void;
Signed-off-by: Sami Tolvanen
---
tools/gendwarfksyms/gendwarfksyms.h | 1 +
tools/gendwarfksyms/types.c | 58 -
2 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/tools/gendwarfksyms/gendwarfksyms.h
b/tools/gendwarfksyms/gendwarfksyms.h
index 43eff91e2f2f..03d8a4a039c3 100644
--- a/tools/gendwarfksyms/gendwarfksyms.h
+++ b/tools/gendwarfksyms/gendwarfksyms.h
@@ -52,6 +52,7 @@ extern bool no_pretty_print;
})
/* Consistent aliases (DW_TAG__type) for DWARF tags */
+#define DW_TAG_formal_parameter_type DW_TAG_formal_parameter
#define DW_TAG_typedef_type DW_TAG_typedef
/*
diff --git a/tools/gendwarfksyms/types.c b/tools/gendwarfksyms/types.c
index 74b3755c3e16..a56aeaa4f3a1 100644
--- a/tools/gendwarfksyms/types.c
+++ b/tools/gendwarfksyms/types.c
@@ -169,6 +169,15 @@ static int process_fqn(struct state *state, struct
cached_die *cache,
DEFINE_PROCESS_UDATA_ATTRIBUTE(alignment)
DEFINE_PROCESS_UDATA_ATTRIBUTE(byte_size)
+/* Match functions -- die_match_callback_t */
+#define DEFINE_MATCH(type) \
+ static bool match_##type##_type(Dwarf_Die *die)\
+ { \
+ return dwarf_tag(die) == DW_TAG_##type##_type; \
+ }
+
+DEFINE_MATCH(formal_parameter)
+
bool match_all(Dwarf_Die *die)
{
return true;
@@ -206,6 +215,25 @@ static int process_type_attr(struct state *state, struct
cached_die *cache,
return check(process(state, cache, "base_type void"));
}
+/* Comma-separated with DW_AT_type */
+static int __process_list_type(struct state *state, struct cached_die *cache,
+ Dwarf_Die *die, const char *type)
+{
+ check(process(state, cache, type));
+ check(process_type_attr(state, cache, die));
+ check(process(state, cache, ","));
+ return check(process_linebreak(cache, 0));
+}
+
+#define DEFINE_PROCESS_LIST_TYPE(type)
\
+ static int process_##type##_type( \
+ struct state *state, struct cached_die *cache, Dwarf_Die *die) \
+ { \
+ return __process_list_type(state, cache, die, #type " "); \
+ }
+
+DEFINE_PROCESS_LIST_TYPE(formal_parameter)
+
/* Container types with DW_AT_type */
static int __process_type(struct state *state, struct cached_die *cache,
Dwarf_Die *die, const char *type)
@@ -240,6 +268,30 @@ DEFINE_PROCESS_TYPE(shared)
DEFINE_PROCESS_TYPE(volatile)
DEFINE_PROCESS_TYPE(typedef)
+static int __process_subroutine_type(struct state *state,
+struct cached_die *cache, Dwarf_Die *die,
+const char *type)
+{
+ check(process(state, cache, type));
+ check(process(state, cache, "("));
+ check(process_linebreak(cache, 1));
+ /* Parameters */
+ check(process_die_container(state, cache, die, process_type,
+ match_formal_parameter_type));
+ check(process_linebreak(cache, -1));
+ check(process(state, cache, ")"));
+ process_linebreak(cache, 0);
+ /* Return type */
+ check(process(state, cache, "-> "));
+ return check(process_type_attr(state, cache, die));
+}
+
+static int process_subroutine_type(struct state *state,
+ struct cached_die *cache, Dwarf_Die *die)
+{
+ return check(__process_subroutine_type(state, cache, die,
+ "subroutine_type"));
+}
static int process_base_type(struct state *state, struct cached_die *cache,
Dwarf_Die *die)
{
@@ -324,8 +376,11 @@ static int process_type(struct state *state, struct
cached_die *parent,
PROCESS_TYPE(rvalue_reference)
PROCESS_TYPE(shared)
PROCESS_TYPE(volatile)
+ /* Subtypes */
+ PROCESS_TYPE(formal_parameter)
/* Other types */
PROCESS_TYPE(base)
+ PROCESS_TYPE(subroutine)
PROCESS_TYPE(typedef)
default:
debug("unimplemented type: %x", tag);
@@ -346,7 +401,8 @@ static int process_type(struct state *state, struct
cached_die *parent,
*/
static int process_subprogram(struct state *state, Dwarf_Die *die)
{
- return check(process(state, NULL, "subprogram;\n"));
+ check(__process_subroutine_type(state, NULL, die, "subpr
[PATCH 07/15] gendwarfksyms: Add pretty-printing
Add linebreaks and indentation to --debug output. This will be
particularly useful for more complex structures.
Example output with --debug:
variable pointer_type {
base_type char byte_size(1)
};
And the same with --debug --no-pretty-print:
variable pointer_type {base_type char byte_size(1)};
Signed-off-by: Sami Tolvanen
---
tools/gendwarfksyms/cache.c | 13 +
tools/gendwarfksyms/gendwarfksyms.c | 3 +++
tools/gendwarfksyms/gendwarfksyms.h | 5 -
tools/gendwarfksyms/types.c | 22 ++
4 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/tools/gendwarfksyms/cache.c b/tools/gendwarfksyms/cache.c
index 9adda113e0b6..85a2adeb649d 100644
--- a/tools/gendwarfksyms/cache.c
+++ b/tools/gendwarfksyms/cache.c
@@ -133,6 +133,19 @@ int cache_add_string(struct cached_die *cd, const char
*str)
return 0;
}
+int cache_add_linebreak(struct cached_die *cd, int linebreak)
+{
+ struct cached_item *ci;
+
+ if (!cd)
+ return 0;
+
+ check(append_item(cd, ));
+ ci->data.linebreak = linebreak;
+ ci->type = LINEBREAK;
+ return 0;
+}
+
int cache_add_die(struct cached_die *cd, Dwarf_Die *die)
{
struct cached_item *ci;
diff --git a/tools/gendwarfksyms/gendwarfksyms.c
b/tools/gendwarfksyms/gendwarfksyms.c
index 38ccaeb72426..7095f0ecccab 100644
--- a/tools/gendwarfksyms/gendwarfksyms.c
+++ b/tools/gendwarfksyms/gendwarfksyms.c
@@ -17,6 +17,8 @@
bool debug;
/* Don't use caching */
bool no_cache;
+/* Don't pretty-print (with --debug) */
+bool no_pretty_print;
static const struct {
const char *arg;
@@ -24,6 +26,7 @@ static const struct {
} options[] = {
{ "--debug", },
{ "--no-cache", _cache },
+ { "--no-pretty-print", _pretty_print },
};
static int usage(void)
diff --git a/tools/gendwarfksyms/gendwarfksyms.h
b/tools/gendwarfksyms/gendwarfksyms.h
index d5ebfe405445..43eff91e2f2f 100644
--- a/tools/gendwarfksyms/gendwarfksyms.h
+++ b/tools/gendwarfksyms/gendwarfksyms.h
@@ -19,6 +19,7 @@
*/
extern bool debug;
extern bool no_cache;
+extern bool no_pretty_print;
/*
* Output helpers
@@ -76,12 +77,13 @@ extern void symbol_print_versions(void);
/*
* cache.c
*/
-enum cached_item_type { EMPTY, STRING, DIE };
+enum cached_item_type { EMPTY, STRING, LINEBREAK, DIE };
struct cached_item {
enum cached_item_type type;
union {
char *str;
+ int linebreak;
uintptr_t addr;
} data;
struct cached_item *next;
@@ -99,6 +101,7 @@ struct cached_die {
extern int cache_get(Dwarf_Die *die, enum cached_die_state state,
struct cached_die **res);
extern int cache_add_string(struct cached_die *pd, const char *str);
+extern int cache_add_linebreak(struct cached_die *pd, int linebreak);
extern int cache_add_die(struct cached_die *pd, Dwarf_Die *die);
extern void cache_free(void);
diff --git a/tools/gendwarfksyms/types.c b/tools/gendwarfksyms/types.c
index 27169c77937f..74b3755c3e16 100644
--- a/tools/gendwarfksyms/types.c
+++ b/tools/gendwarfksyms/types.c
@@ -6,6 +6,17 @@
#include "gendwarfksyms.h"
#include "crc32.h"
+static bool do_linebreak;
+static int indentation_level;
+
+/* Line breaks and indentation for pretty-printing */
+static int process_linebreak(struct cached_die *cache, int n)
+{
+ indentation_level += n;
+ do_linebreak = true;
+ return check(cache_add_linebreak(cache, n));
+}
+
#define DEFINE_GET_ATTR(attr, type)\
static bool get_##attr##_attr(Dwarf_Die *die, unsigned int id, \
type *value) \
@@ -76,6 +87,12 @@ static int process(struct state *state, struct cached_die
*cache, const char *s)
{
s = s ?: "";
+ if (debug && !no_pretty_print && do_linebreak) {
+ fputs("\n", stderr);
+ for (int i = 0; i < indentation_level; i++)
+ fputs(" ", stderr);
+ do_linebreak = false;
+ }
if (debug)
fputs(s, stderr);
@@ -196,7 +213,9 @@ static int __process_type(struct state *state, struct
cached_die *cache,
check(process(state, cache, type));
check(process_fqn(state, cache, die));
check(process(state, cache, " {"));
+ check(process_linebreak(cache, 1));
check(process_type_attr(state, cache, die));
+ check(process_linebreak(cache, -1));
check(process(state, cache, "}"));
check(process_byte_size_attr(state, cache, die));
return check(process_alignment_attr(state, cache, die));
@@ -241,6 +260,9 @@ static int process_cached(struct state *state, struct
cached_die *cache,
case STRING:
check(proc
[PATCH 06/15] gendwarfksyms: Expand type modifiers and typedefs
Add support for expanding DWARF type modifiers, such as pointers,
const values etc., and typedefs. These types all have DW_AT_type
attribute pointing the underlying type, and thus produce similar
output.
Signed-off-by: Sami Tolvanen
---
tools/gendwarfksyms/gendwarfksyms.h | 3 ++
tools/gendwarfksyms/types.c | 54 +++--
2 files changed, 54 insertions(+), 3 deletions(-)
diff --git a/tools/gendwarfksyms/gendwarfksyms.h
b/tools/gendwarfksyms/gendwarfksyms.h
index ea5a9fbda66f..d5ebfe405445 100644
--- a/tools/gendwarfksyms/gendwarfksyms.h
+++ b/tools/gendwarfksyms/gendwarfksyms.h
@@ -50,6 +50,9 @@ extern bool no_cache;
__res; \
})
+/* Consistent aliases (DW_TAG__type) for DWARF tags */
+#define DW_TAG_typedef_type DW_TAG_typedef
+
/*
* symbols.c
*/
diff --git a/tools/gendwarfksyms/types.c b/tools/gendwarfksyms/types.c
index 78046c08be23..27169c77937f 100644
--- a/tools/gendwarfksyms/types.c
+++ b/tools/gendwarfksyms/types.c
@@ -189,6 +189,38 @@ static int process_type_attr(struct state *state, struct
cached_die *cache,
return check(process(state, cache, "base_type void"));
}
+/* Container types with DW_AT_type */
+static int __process_type(struct state *state, struct cached_die *cache,
+ Dwarf_Die *die, const char *type)
+{
+ check(process(state, cache, type));
+ check(process_fqn(state, cache, die));
+ check(process(state, cache, " {"));
+ check(process_type_attr(state, cache, die));
+ check(process(state, cache, "}"));
+ check(process_byte_size_attr(state, cache, die));
+ return check(process_alignment_attr(state, cache, die));
+}
+
+#define DEFINE_PROCESS_TYPE(type)
\
+ static int process_##type##_type( \
+ struct state *state, struct cached_die *cache, Dwarf_Die *die) \
+ { \
+ return __process_type(state, cache, die, #type "_type "); \
+ }
+
+DEFINE_PROCESS_TYPE(atomic)
+DEFINE_PROCESS_TYPE(const)
+DEFINE_PROCESS_TYPE(immutable)
+DEFINE_PROCESS_TYPE(packed)
+DEFINE_PROCESS_TYPE(pointer)
+DEFINE_PROCESS_TYPE(reference)
+DEFINE_PROCESS_TYPE(restrict)
+DEFINE_PROCESS_TYPE(rvalue_reference)
+DEFINE_PROCESS_TYPE(shared)
+DEFINE_PROCESS_TYPE(volatile)
+DEFINE_PROCESS_TYPE(typedef)
+
static int process_base_type(struct state *state, struct cached_die *cache,
Dwarf_Die *die)
{
@@ -233,6 +265,11 @@ static void state_init(struct state *state)
state->crc = 0x;
}
+#define PROCESS_TYPE(type) \
+ case DW_TAG_##type##_type: \
+ check(process_##type##_type(state, cache, die)); \
+ break;
+
static int process_type(struct state *state, struct cached_die *parent,
Dwarf_Die *die)
{
@@ -254,9 +291,20 @@ static int process_type(struct state *state, struct
cached_die *parent,
}
switch (tag) {
- case DW_TAG_base_type:
- check(process_base_type(state, cache, die));
- break;
+ /* Type modifiers */
+ PROCESS_TYPE(atomic)
+ PROCESS_TYPE(const)
+ PROCESS_TYPE(immutable)
+ PROCESS_TYPE(packed)
+ PROCESS_TYPE(pointer)
+ PROCESS_TYPE(reference)
+ PROCESS_TYPE(restrict)
+ PROCESS_TYPE(rvalue_reference)
+ PROCESS_TYPE(shared)
+ PROCESS_TYPE(volatile)
+ /* Other types */
+ PROCESS_TYPE(base)
+ PROCESS_TYPE(typedef)
default:
debug("unimplemented type: %x", tag);
break;
--
2.45.2.627.g7a2c4fd464-goog
[PATCH 05/15] gendwarfksyms: Add a cache
Basic types in DWARF repeat frequently and traversing the DIEs using
libdw is relatively slow. Add a simple hashtable based cache for the
processed DIEs.
Signed-off-by: Sami Tolvanen
---
tools/gendwarfksyms/Build | 1 +
tools/gendwarfksyms/cache.c | 147
tools/gendwarfksyms/gendwarfksyms.c | 4 +
tools/gendwarfksyms/gendwarfksyms.h | 37 ++-
tools/gendwarfksyms/types.c | 140 ++
5 files changed, 286 insertions(+), 43 deletions(-)
create mode 100644 tools/gendwarfksyms/cache.c
diff --git a/tools/gendwarfksyms/Build b/tools/gendwarfksyms/Build
index 518642c9b9de..220a4aa9b380 100644
--- a/tools/gendwarfksyms/Build
+++ b/tools/gendwarfksyms/Build
@@ -1,4 +1,5 @@
gendwarfksyms-y += gendwarfksyms.o
+gendwarfksyms-y += cache.o
gendwarfksyms-y += crc32.o
gendwarfksyms-y += symbols.o
gendwarfksyms-y += types.o
diff --git a/tools/gendwarfksyms/cache.c b/tools/gendwarfksyms/cache.c
new file mode 100644
index ..9adda113e0b6
--- /dev/null
+++ b/tools/gendwarfksyms/cache.c
@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 Google LLC
+ */
+
+#include
+#include "gendwarfksyms.h"
+
+#define DIE_HASH_BITS 10
+
+/* die->addr -> struct cached_die */
+DEFINE_HASHTABLE(die_cache, DIE_HASH_BITS);
+
+static unsigned int cache_hits;
+static unsigned int cache_misses;
+
+static int create_die(Dwarf_Die *die, struct cached_die **res)
+{
+ struct cached_die *cd;
+
+ cd = malloc(sizeof(struct cached_die));
+ if (!cd) {
+ error("malloc failed");
+ return -1;
+ }
+
+ cd->state = INCOMPLETE;
+ cd->addr = (uintptr_t)die->addr;
+ cd->list = NULL;
+
+ hash_add(die_cache, >hash, cd->addr);
+ *res = cd;
+ return 0;
+}
+
+int cache_get(Dwarf_Die *die, enum cached_die_state state,
+ struct cached_die **res)
+{
+ struct cached_die *cd;
+ uintptr_t addr = (uintptr_t)die->addr;
+
+ hash_for_each_possible(die_cache, cd, hash, addr) {
+ if (cd->addr == addr && cd->state == state) {
+ *res = cd;
+ cache_hits++;
+ return 0;
+ }
+ }
+
+ cache_misses++;
+ return check(create_die(die, res));
+}
+
+static void reset_die(struct cached_die *cd)
+{
+ struct cached_item *tmp;
+ struct cached_item *ci = cd->list;
+
+ while (ci) {
+ if (ci->type == STRING)
+ free(ci->data.str);
+
+ tmp = ci->next;
+ free(ci);
+ ci = tmp;
+ }
+
+ cd->state = INCOMPLETE;
+ cd->list = NULL;
+}
+
+void cache_free(void)
+{
+ struct cached_die *cd;
+ struct hlist_node *tmp;
+ int i;
+
+ hash_for_each_safe(die_cache, i, tmp, cd, hash) {
+ reset_die(cd);
+ free(cd);
+ }
+ hash_init(die_cache);
+
+ if ((cache_hits + cache_misses > 0))
+ debug("cache: hits %u, misses %u (hit rate %.02f%%)",
+ cache_hits, cache_misses,
+ (100.0f * cache_hits) / (cache_hits + cache_misses));
+}
+
+static int append_item(struct cached_die *cd, struct cached_item **res)
+{
+ struct cached_item *prev;
+ struct cached_item *ci;
+
+ ci = malloc(sizeof(struct cached_item));
+ if (!ci) {
+ error("malloc failed");
+ return -1;
+ }
+
+ ci->type = EMPTY;
+ ci->next = NULL;
+
+ prev = cd->list;
+ while (prev && prev->next)
+ prev = prev->next;
+
+ if (prev)
+ prev->next = ci;
+ else
+ cd->list = ci;
+
+ *res = ci;
+ return 0;
+}
+
+int cache_add_string(struct cached_die *cd, const char *str)
+{
+ struct cached_item *ci;
+
+ if (!cd)
+ return 0;
+
+ check(append_item(cd, ));
+
+ ci->data.str = strdup(str);
+ if (!ci->data.str) {
+ error("strdup failed");
+ return -1;
+ }
+
+ ci->type = STRING;
+ return 0;
+}
+
+int cache_add_die(struct cached_die *cd, Dwarf_Die *die)
+{
+ struct cached_item *ci;
+
+ if (!cd)
+ return 0;
+
+ check(append_item(cd, ));
+ ci->data.addr = (uintptr_t)die->addr;
+ ci->type = DIE;
+ return 0;
+}
diff --git a/tools/gendwarfksyms/gendwarfksyms.c
b/tools/gendwarfksyms/gendwarfksyms.c
index 7938b7440674..38ccaeb72426 100644
--- a/tools/gendwarfksyms/gendwarfksyms.c
+++ b/tools/gendwarfksyms/gendwarfksyms.c
@@ -15,12 +15,15 @@
/* Print type descriptions and debugging output to stderr */
bool debug;
+/* Don't use caching */
+bool no_cache;
s
[PATCH 04/15] gendwarfksyms: Expand base_type
Add support for expanding basic DWARF attributes and DW_TAG_base_type
types.
Example usage:
$ nm -p --defined-only vmlinux.o | \
awk '/ (T|R|D) / {printf "%s %s\n",$1, $3}' | \
grep loops_per_jiffy | \
./tools/gendwarfksyms/gendwarfksyms --debug vmlinux.o
Output:
gendwarfksyms: symbol_read_list: adding { 4b0, "loops_per_jiffy" }
gendwarfksyms: process_modules: vmlinux.o
gendwarfksyms: process_exported_symbols: loops_per_jiffy (@ 4b0)
variable base_type unsigned long byte_size(8);
#SYMVER loops_per_jiffy 0xc694aefc
Signed-off-by: Sami Tolvanen
---
tools/gendwarfksyms/types.c | 109 +++-
1 file changed, 108 insertions(+), 1 deletion(-)
diff --git a/tools/gendwarfksyms/types.c b/tools/gendwarfksyms/types.c
index 47ae967d42ee..7508d0fb8b80 100644
--- a/tools/gendwarfksyms/types.c
+++ b/tools/gendwarfksyms/types.c
@@ -16,6 +16,7 @@
}
DEFINE_GET_ATTR(addr, Dwarf_Addr)
+DEFINE_GET_ATTR(udata, Dwarf_Word)
static bool get_ref_die_attr(Dwarf_Die *die, unsigned int id, Dwarf_Die *value)
{
@@ -82,6 +83,73 @@ static int process(struct state *state, const char *s)
return 0;
}
+#define MAX_FMT_BUFFER_SIZE 128
+
+static int process_fmt(struct state *state, const char *fmt, ...)
+{
+ char buf[MAX_FMT_BUFFER_SIZE];
+ va_list args;
+ int res;
+
+ va_start(args, fmt);
+
+ res = check(vsnprintf(buf, sizeof(buf), fmt, args));
+ if (res >= MAX_FMT_BUFFER_SIZE - 1) {
+ error("vsnprintf overflow: increase MAX_FMT_BUFFER_SIZE");
+ res = -1;
+ } else {
+ check(process(state, buf));
+ }
+
+ va_end(args);
+ return res;
+}
+
+/* Process a fully qualified name from DWARF scopes */
+static int process_fqn(struct state *state, Dwarf_Die *die)
+{
+ Dwarf_Die *scopes = NULL;
+ const char *name;
+ int res;
+ int i;
+
+ res = check(dwarf_getscopes_die(die, ));
+ if (!res) {
+ name = get_name(die);
+ name = name ?: "";
+ return check(process(state, name));
+ }
+
+ for (i = res - 1; i >= 0; i--) {
+ if (dwarf_tag([i]) == DW_TAG_compile_unit)
+ continue;
+
+ name = get_name([i]);
+ name = name ?: "";
+ check(process(state, name));
+ if (i > 0)
+ check(process(state, "::"));
+ }
+
+ free(scopes);
+ return 0;
+}
+
+#define DEFINE_PROCESS_UDATA_ATTRIBUTE(attribute) \
+ static int process_##attribute##_attr(struct state *state,\
+ Dwarf_Die *die) \
+ { \
+ Dwarf_Word value; \
+ if (get_udata_attr(die, DW_AT_##attribute, )) \
+ check(process_fmt(state, \
+ " " #attribute "(%" PRIu64 ")", \
+ value));\
+ return 0; \
+ }
+
+DEFINE_PROCESS_UDATA_ATTRIBUTE(alignment)
+DEFINE_PROCESS_UDATA_ATTRIBUTE(byte_size)
+
bool match_all(Dwarf_Die *die)
{
return true;
@@ -103,11 +171,48 @@ int process_die_container(struct state *state, Dwarf_Die
*die,
return 0;
}
+static int process_type(struct state *state, Dwarf_Die *die);
+
+static int process_type_attr(struct state *state, Dwarf_Die *die)
+{
+ Dwarf_Die type;
+
+ if (get_ref_die_attr(die, DW_AT_type, ))
+ return check(process_type(state, ));
+
+ /* Compilers can omit DW_AT_type -- print out 'void' to clarify */
+ return check(process(state, "base_type void"));
+}
+
+static int process_base_type(struct state *state, Dwarf_Die *die)
+{
+ check(process(state, "base_type "));
+ check(process_fqn(state, die));
+ check(process_byte_size_attr(state, die));
+ return check(process_alignment_attr(state, die));
+}
+
static void state_init(struct state *state)
{
state->crc = 0x;
}
+static int process_type(struct state *state, Dwarf_Die *die)
+{
+ int tag = dwarf_tag(die);
+
+ switch (tag) {
+ case DW_TAG_base_type:
+ check(process_base_type(state, die));
+ break;
+ default:
+ debug("unimplemented type: %x", tag);
+ break;
+ }
+
+ return 0;
+}
+
/*
* Exported symbol processing
*/
@@ -118,7 +223,9 @@ static int process_subprogram(struct state *state,
Dwarf_Die *die)
static int process_variable(struct state *state, Dwarf_Die *die)
[PATCH 03/15] gendwarfksyms: Add CRC calculation
Add a basic CRC32 implementation adapted from genksyms, and produce
matching output from type strings.
Example usage:
$ nm -p --defined-only rust/alloc.o | \
awk '/ (T|R|D) / {printf "%s %s\n",$1, $3}' | \
tools/gendwarfksyms/gendwarfksyms rust/alloc.o
Output:
#SYMVER
_RNvNvMs_NtCscg2doA4GLaz_5alloc3vecINtB6_3VecppE11swap_remove13assert_failed
0x985f94dd
#SYMVER
_RNvXs1_NtCscg2doA4GLaz_5alloc11collectionsNtB5_15TryReserveErrorNtNtCs5QSdWC790r4_4core3fmt7Display3fmt
0x985f94dd
#SYMVER
_RNvNvMs_NtCscg2doA4GLaz_5alloc3vecINtB6_3VecppE6remove13assert_failed
0x985f94dd
...
Note that type strings are not yet expanded, so the CRCs can only
distinguish between functions and variables for now.
Signed-off-by: Sami Tolvanen
---
tools/gendwarfksyms/Build | 1 +
tools/gendwarfksyms/crc32.c | 69
tools/gendwarfksyms/crc32.h | 34 ++
tools/gendwarfksyms/gendwarfksyms.c | 1 +
tools/gendwarfksyms/gendwarfksyms.h | 6 +++
tools/gendwarfksyms/symbols.c | 71 +++--
tools/gendwarfksyms/types.c | 19 ++--
7 files changed, 194 insertions(+), 7 deletions(-)
create mode 100644 tools/gendwarfksyms/crc32.c
create mode 100644 tools/gendwarfksyms/crc32.h
diff --git a/tools/gendwarfksyms/Build b/tools/gendwarfksyms/Build
index a83c59bfef8b..518642c9b9de 100644
--- a/tools/gendwarfksyms/Build
+++ b/tools/gendwarfksyms/Build
@@ -1,3 +1,4 @@
gendwarfksyms-y += gendwarfksyms.o
+gendwarfksyms-y += crc32.o
gendwarfksyms-y += symbols.o
gendwarfksyms-y += types.o
diff --git a/tools/gendwarfksyms/crc32.c b/tools/gendwarfksyms/crc32.c
new file mode 100644
index ..23b328cd74f2
--- /dev/null
+++ b/tools/gendwarfksyms/crc32.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Based on scripts/genksyms/genksyms.c, which has the following
+ * notice:
+ *
+ * Generate kernel symbol version hashes.
+ * Copyright 1996, 1997 Linux International.
+ *
+ * New implementation contributed by Richard Henderson
+ * Based on original work by Bjorn Ekwall
+ *
+ * This file was part of the Linux modutils 2.4.22: moved back into the
+ * kernel sources by Rusty Russell/Kai Germaschewski.
+ */
+
+const unsigned int crctab32[] = {
+ 0xU, 0x77073096U, 0xee0e612cU, 0x990951baU, 0x076dc419U,
+ 0x706af48fU, 0xe963a535U, 0x9e6495a3U, 0x0edb8832U, 0x79dcb8a4U,
+ 0xe0d5e91eU, 0x97d2d988U, 0x09b64c2bU, 0x7eb17cbdU, 0xe7b82d07U,
+ 0x90bf1d91U, 0x1db71064U, 0x6ab020f2U, 0xf3b97148U, 0x84be41deU,
+ 0x1adad47dU, 0x6ddde4ebU, 0xf4d4b551U, 0x83d385c7U, 0x136c9856U,
+ 0x646ba8c0U, 0xfd62f97aU, 0x8a65c9ecU, 0x14015c4fU, 0x63066cd9U,
+ 0xfa0f3d63U, 0x8d080df5U, 0x3b6e20c8U, 0x4c69105eU, 0xd56041e4U,
+ 0xa2677172U, 0x3c03e4d1U, 0x4b04d447U, 0xd20d85fdU, 0xa50ab56bU,
+ 0x35b5a8faU, 0x42b2986cU, 0xdbbbc9d6U, 0xacbcf940U, 0x32d86ce3U,
+ 0x45df5c75U, 0xdcd60dcfU, 0xabd13d59U, 0x26d930acU, 0x51de003aU,
+ 0xc8d75180U, 0xbfd06116U, 0x21b4f4b5U, 0x56b3c423U, 0xcfba9599U,
+ 0xb8bda50fU, 0x2802b89eU, 0x5f058808U, 0xc60cd9b2U, 0xb10be924U,
+ 0x2f6f7c87U, 0x58684c11U, 0xc1611dabU, 0xb6662d3dU, 0x76dc4190U,
+ 0x01db7106U, 0x98d220bcU, 0xefd5102aU, 0x71b18589U, 0x06b6b51fU,
+ 0x9fbfe4a5U, 0xe8b8d433U, 0x7807c9a2U, 0x0f00f934U, 0x9609a88eU,
+ 0xe10e9818U, 0x7f6a0dbbU, 0x086d3d2dU, 0x91646c97U, 0xe6635c01U,
+ 0x6b6b51f4U, 0x1c6c6162U, 0x856530d8U, 0xf262004eU, 0x6c0695edU,
+ 0x1b01a57bU, 0x8208f4c1U, 0xf50fc457U, 0x65b0d9c6U, 0x12b7e950U,
+ 0x8bbeb8eaU, 0xfcb9887cU, 0x62dd1ddfU, 0x15da2d49U, 0x8cd37cf3U,
+ 0xfbd44c65U, 0x4db26158U, 0x3ab551ceU, 0xa3bc0074U, 0xd4bb30e2U,
+ 0x4adfa541U, 0x3dd895d7U, 0xa4d1c46dU, 0xd3d6f4fbU, 0x4369e96aU,
+ 0x346ed9fcU, 0xad678846U, 0xda60b8d0U, 0x44042d73U, 0x33031de5U,
+ 0xaa0a4c5fU, 0xdd0d7cc9U, 0x5005713cU, 0x270241aaU, 0xbe0b1010U,
+ 0xc90c2086U, 0x5768b525U, 0x206f85b3U, 0xb966d409U, 0xce61e49fU,
+ 0x5edef90eU, 0x29d9c998U, 0xb0d09822U, 0xc7d7a8b4U, 0x59b33d17U,
+ 0x2eb40d81U, 0xb7bd5c3bU, 0xc0ba6cadU, 0xedb88320U, 0x9abfb3b6U,
+ 0x03b6e20cU, 0x74b1d29aU, 0xead54739U, 0x9dd277afU, 0x04db2615U,
+ 0x73dc1683U, 0xe3630b12U, 0x94643b84U, 0x0d6d6a3eU, 0x7a6a5aa8U,
+ 0xe40ecf0bU, 0x9309ff9dU, 0x0a00ae27U, 0x7d079eb1U, 0xf00f9344U,
+ 0x8708a3d2U, 0x1e01f268U, 0x6906c2feU, 0xf762575dU, 0x806567cbU,
+ 0x196c3671U, 0x6e6b06e7U, 0xfed41b76U, 0x89d32be0U, 0x10da7a5aU,
+ 0x67dd4accU, 0xf9b9df6fU, 0x8ebeeff9U, 0x17b7be43U, 0x60b08ed5U,
+ 0xd6d6a3e8U, 0xa1d1937eU, 0x38d8c2c4U, 0x4fdff252U, 0xd1bb67f1U,
+ 0xa6bc5767U, 0x3fb506ddU, 0x48b2364bU, 0xd80d2bdaU, 0xaf0a1b4cU,
+ 0x36034af6U, 0x41047a60U, 0xdf60efc3U, 0xa867df55U, 0x316e8eefU,
+ 0x4669be79U, 0xcb61b38cU, 0xbc66831aU, 0x256fd2a0U, 0x5268e236U,
+ 0xcc0c7795U, 0xbb0b4703U, 0x220216b9U, 0x5505262fU, 0
[PATCH 02/15] gendwarfksyms: Add symbol list input handling
Support passing a list of exported symbols to gendwarfksyms via stdin
and filter non-exported symbols from the output.
The symbol list input has the format 'symbol-address symbol-name' to
allow the parser to discover exported symbols also by address. This
is necessary for aliased symbols, where only one name appears in the
debugging information.
Signed-off-by: Sami Tolvanen
---
tools/gendwarfksyms/Build | 1 +
tools/gendwarfksyms/gendwarfksyms.c | 2 +
tools/gendwarfksyms/gendwarfksyms.h | 17
tools/gendwarfksyms/symbols.c | 130
tools/gendwarfksyms/types.c | 70 ++-
5 files changed, 219 insertions(+), 1 deletion(-)
create mode 100644 tools/gendwarfksyms/symbols.c
diff --git a/tools/gendwarfksyms/Build b/tools/gendwarfksyms/Build
index 805591b6df80..a83c59bfef8b 100644
--- a/tools/gendwarfksyms/Build
+++ b/tools/gendwarfksyms/Build
@@ -1,2 +1,3 @@
gendwarfksyms-y += gendwarfksyms.o
+gendwarfksyms-y += symbols.o
gendwarfksyms-y += types.o
diff --git a/tools/gendwarfksyms/gendwarfksyms.c
b/tools/gendwarfksyms/gendwarfksyms.c
index 4a2dea307849..4a1bd9239182 100644
--- a/tools/gendwarfksyms/gendwarfksyms.c
+++ b/tools/gendwarfksyms/gendwarfksyms.c
@@ -96,6 +96,8 @@ int main(int argc, const char **argv)
if (parse_options(argc, argv, ) < 0)
return usage();
+ check(symbol_read_list(stdin));
+
fd = open(filename, O_RDONLY);
if (fd == -1) {
error("open failed for '%s': %s", filename, strerror(errno));
diff --git a/tools/gendwarfksyms/gendwarfksyms.h
b/tools/gendwarfksyms/gendwarfksyms.h
index 44e94f1d9671..b77855cc94a7 100644
--- a/tools/gendwarfksyms/gendwarfksyms.h
+++ b/tools/gendwarfksyms/gendwarfksyms.h
@@ -49,6 +49,21 @@ extern bool debug;
__res; \
})
+/*
+ * symbols.c
+ */
+
+/* Exported symbol -- matching either the name or the address */
+struct symbol {
+ const char *name;
+ uintptr_t addr;
+ struct hlist_node addr_hash;
+ struct hlist_node name_hash;
+};
+
+extern int symbol_read_list(FILE *file);
+extern struct symbol *symbol_get(uintptr_t addr, const char *name);
+
/*
* types.c
*/
@@ -56,6 +71,8 @@ extern bool debug;
struct state {
Dwfl_Module *mod;
Dwarf *dbg;
+ struct symbol *sym;
+ Dwarf_Die origin;
};
typedef int (*die_callback_t)(struct state *state, Dwarf_Die *die);
diff --git a/tools/gendwarfksyms/symbols.c b/tools/gendwarfksyms/symbols.c
new file mode 100644
index ..2cae61bcede7
--- /dev/null
+++ b/tools/gendwarfksyms/symbols.c
@@ -0,0 +1,130 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 Google LLC
+ */
+
+#include
+#include
+#include "gendwarfksyms.h"
+
+/* Hash tables for looking up requested symbols by address and name */
+#define SYMBOL_HASH_BITS 7
+DEFINE_HASHTABLE(symbol_addrs, SYMBOL_HASH_BITS);
+DEFINE_HASHTABLE(symbol_names, SYMBOL_HASH_BITS);
+
+static u32 name_hash(const char *name)
+{
+ return jhash(name, strlen(name), 0);
+}
+
+/* symbol_for_each callback -- return true to stop, false to continue */
+typedef bool (*symbol_callback_t)(struct symbol *, void *arg);
+
+static bool for_each_addr(uintptr_t addr, symbol_callback_t func, void *data)
+{
+ struct symbol *sym;
+ bool found = false;
+
+ if (addr == UINTPTR_MAX)
+ return false;
+
+ hash_for_each_possible(symbol_addrs, sym, addr_hash, addr) {
+ if (sym->addr == addr) {
+ if (func(sym, data))
+ return true;
+ found = true;
+ }
+ }
+
+ return found;
+}
+
+static bool for_each_name(const char *name, symbol_callback_t func, void *data)
+{
+ struct symbol *sym;
+ bool found = false;
+
+ if (!name)
+ return false;
+
+ hash_for_each_possible(symbol_names, sym, name_hash, name_hash(name)) {
+ if (!strcmp(sym->name, name)) {
+ if (func(sym, data))
+ return true;
+ found = true;
+ }
+ }
+
+ return found;
+}
+
+static bool for_each(uintptr_t addr, const char *name, symbol_callback_t func,
+void *data)
+{
+ bool found = false;
+
+ if (for_each_addr(addr, func, data))
+ found = true;
+ if (for_each_name(name, func, data))
+ found = true;
+
+ return found;
+}
+
+int symbol_read_list(FILE *file)
+{
+ struct symbol *sym;
+ char *line = NULL;
+ char *name = NULL;
+ uint64_t addr;
+ size_t size = 0;
+
+ while (getline(, , file) > 0) {
+ if (sscanf(line, "%" PRIx64 " %ms\n", , ) != 2) {
+ error("m
[PATCH 00/15] Implement MODVERSIONS for Rust
Hi folks, This series implements CONFIG_MODVERSIONS for Rust, an important feature for distributions like Android that want to ship Rust kernel modules, and depend on modversions to help ensure module ABI compatibility. There have been earlier proposals [1][2] that would allow Rust modules to coexist with modversions, but none that actually implement symbol versioning. Unlike C, Rust source code doesn't have sufficient information about the final ABI, as the compiler has considerable freedom in adjusting structure layout for improved performance [3], for example, which makes using a source code parser like genksyms a non-starter. Based on Matt's suggestion and previous feedback from maintainers, this series uses DWARF debugging information for computing versions. DWARF is an established and relatively stable format, which includes all the necessary ABI details, and adding a CONFIG_DEBUG_INFO dependency for Rust symbol versioning seems like a reasonable trade-off. The first 12 patches of this series add a small tool for computing symbol versions from DWARF, called gendwarfksyms. When passed a list of exported symbols, the tool generates an expanded type string for each symbol, and computes symbol CRCs similarly to genksyms. gendwarfksyms is written in C and uses libdw to process DWARF, mainly because of the existing support for C host tools that use elfutils (e.g., objtool). Another compatibility issue is fitting the extremely long mangled Rust symbol names into struct modversion_info, which can only hold 55-character names (on 64-bit systems). Previous proposals suggested changing the structure to support longer names, but the conclusion was that we cannot break userspace tools that parse the version table. The next two patches of the series implement support for hashed symbol names in struct modversion_info, where names longer than 55 characters are hashed, and the hash is stored in the name field. To avoid breaking userspace tools, the binary hash is prefixed with a null-terminated string containing the name of the hash function. While userspace tools can later be updated to potentially produce more useful information about the long symbols, this allows them to continue working in the meantime. The final patch allows CONFIG_MODVERSIONS to be selected with Rust, provided that debugging information is also available. [1] https://lore.kernel.org/lkml/2023061155.1349375-1-g...@garyguo.net/ [2] https://lore.kernel.org/rust-for-linux/20231118025748.2778044-1-mmau...@google.com/ [3] https://lore.kernel.org/rust-for-linux/cagsqo005hriuzdeppcifdqg9zfdjrwahpble4x7-myfjscn...@mail.gmail.com/ Sami Sami Tolvanen (15): tools: Add gendwarfksyms gendwarfksyms: Add symbol list input handling gendwarfksyms: Add CRC calculation gendwarfksyms: Expand base_type gendwarfksyms: Add a cache gendwarfksyms: Expand type modifiers and typedefs gendwarfksyms: Add pretty-printing gendwarfksyms: Expand subroutine_type gendwarfksyms: Expand array_type gendwarfksyms: Expand structure types gendwarfksyms: Limit structure expansion gendwarfksyms: Add inline debugging modpost: Add support for hashing long symbol names module: Support hashed symbol names when checking modversions kbuild: Use gendwarfksyms to generate Rust symbol versions Makefile| 6 + init/Kconfig| 2 +- kernel/module/version.c | 38 +- rust/Makefile | 30 +- scripts/mod/Makefile| 4 +- scripts/mod/modpost.c | 20 +- scripts/mod/modpost.h | 20 + scripts/mod/symhash.c | 327 + tools/Makefile | 11 +- tools/gendwarfksyms/Build | 5 + tools/gendwarfksyms/Makefile| 49 ++ tools/gendwarfksyms/cache.c | 209 + tools/gendwarfksyms/crc32.c | 69 +++ tools/gendwarfksyms/crc32.h | 34 ++ tools/gendwarfksyms/gendwarfksyms.c | 141 ++ tools/gendwarfksyms/gendwarfksyms.h | 173 +++ tools/gendwarfksyms/symbols.c | 193 tools/gendwarfksyms/types.c | 697 18 files changed, 2008 insertions(+), 20 deletions(-) create mode 100644 scripts/mod/symhash.c create mode 100644 tools/gendwarfksyms/Build create mode 100644 tools/gendwarfksyms/Makefile create mode 100644 tools/gendwarfksyms/cache.c create mode 100644 tools/gendwarfksyms/crc32.c create mode 100644 tools/gendwarfksyms/crc32.h create mode 100644 tools/gendwarfksyms/gendwarfksyms.c create mode 100644 tools/gendwarfksyms/gendwarfksyms.h create mode 100644 tools/gendwarfksyms/symbols.c create mode 100644 tools/gendwarfksyms/types.c base-commit: 6ba59ff4227927d3a8530fc2973b80e94b54d58f -- 2.45.2.627.g7a2c4fd464-goog
[PATCH 01/15] tools: Add gendwarfksyms
Add a basic DWARF parser, which uses libdw to traverse the debugging information in an object file and looks for functions and variables. In follow-up patches, this will be expanded to produce symbol versions for CONFIG_MODVERSIONS from DWARF. This tool is needed mainly for Rust kernel module compatibility. Signed-off-by: Sami Tolvanen --- tools/Makefile | 11 +-- tools/gendwarfksyms/Build | 2 + tools/gendwarfksyms/Makefile| 49 +++ tools/gendwarfksyms/gendwarfksyms.c | 128 tools/gendwarfksyms/gendwarfksyms.h | 71 +++ tools/gendwarfksyms/types.c | 87 +++ 6 files changed, 343 insertions(+), 5 deletions(-) create mode 100644 tools/gendwarfksyms/Build create mode 100644 tools/gendwarfksyms/Makefile create mode 100644 tools/gendwarfksyms/gendwarfksyms.c create mode 100644 tools/gendwarfksyms/gendwarfksyms.h create mode 100644 tools/gendwarfksyms/types.c diff --git a/tools/Makefile b/tools/Makefile index 276f5d0d53a4..60806ff753b7 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -17,6 +17,7 @@ help: @echo ' firewire - the userspace part of nosy, an IEEE-1394 traffic sniffer' @echo ' firmware - Firmware tools' @echo ' freefall - laptop accelerometer program for disk protection' + @echo ' gendwarfksyms - generates symbol versions from DWARF' @echo ' gpio - GPIO tools' @echo ' hv - tools used when in Hyper-V clients' @echo ' iio- IIO tools' @@ -68,7 +69,7 @@ acpi: FORCE cpupower: FORCE $(call descend,power/$@) -counter firewire hv guest bootconfig spi usb virtio mm bpf iio gpio objtool leds wmi pci firmware debugging tracing: FORCE +counter firewire hv guest bootconfig spi usb virtio mm bpf iio gendwarfksyms gpio objtool leds wmi pci firmware debugging tracing: FORCE $(call descend,$@) bpf/%: FORCE @@ -154,8 +155,8 @@ freefall_install: kvm_stat_install: $(call descend,kvm/$(@:_install=),install) -install: acpi_install counter_install cpupower_install gpio_install \ - hv_install firewire_install iio_install \ +install: acpi_install counter_install cpupower_install gendwarfksyms_install \ + gpio_install hv_install firewire_install iio_install \ perf_install selftests_install turbostat_install usb_install \ virtio_install mm_install bpf_install x86_energy_perf_policy_install \ tmon_install freefall_install objtool_install kvm_stat_install \ @@ -168,7 +169,7 @@ acpi_clean: cpupower_clean: $(call descend,power/cpupower,clean) -counter_clean hv_clean firewire_clean bootconfig_clean spi_clean usb_clean virtio_clean mm_clean wmi_clean bpf_clean iio_clean gpio_clean objtool_clean leds_clean pci_clean firmware_clean debugging_clean tracing_clean: +counter_clean hv_clean firewire_clean bootconfig_clean spi_clean usb_clean virtio_clean mm_clean wmi_clean bpf_clean iio_clean gendwarfksyms_clean gpio_clean objtool_clean leds_clean pci_clean firmware_clean debugging_clean tracing_clean: $(call descend,$(@:_clean=),clean) libapi_clean: @@ -211,7 +212,7 @@ build_clean: clean: acpi_clean counter_clean cpupower_clean hv_clean firewire_clean \ perf_clean selftests_clean turbostat_clean bootconfig_clean spi_clean usb_clean virtio_clean \ mm_clean bpf_clean iio_clean x86_energy_perf_policy_clean tmon_clean \ - freefall_clean build_clean libbpf_clean libsubcmd_clean \ + freefall_clean build_clean libbpf_clean libsubcmd_clean gendwarfksyms_clean \ gpio_clean objtool_clean leds_clean wmi_clean pci_clean firmware_clean debugging_clean \ intel-speed-select_clean tracing_clean thermal_clean thermometer_clean thermal-engine_clean diff --git a/tools/gendwarfksyms/Build b/tools/gendwarfksyms/Build new file mode 100644 index ..805591b6df80 --- /dev/null +++ b/tools/gendwarfksyms/Build @@ -0,0 +1,2 @@ +gendwarfksyms-y += gendwarfksyms.o +gendwarfksyms-y += types.o diff --git a/tools/gendwarfksyms/Makefile b/tools/gendwarfksyms/Makefile new file mode 100644 index ..1138ebd8bd0f --- /dev/null +++ b/tools/gendwarfksyms/Makefile @@ -0,0 +1,49 @@ +# SPDX-License-Identifier: GPL-2.0 +include ../scripts/Makefile.include +include ../scripts/Makefile.arch + +ifeq ($(srctree),) +srctree := $(patsubst %/,%,$(dir $(CURDIR))) +srctree := $(patsubst %/,%,$(dir $(srctree))) +endif + +GENDWARFKSYMS:= $(OUTPUT)gendwarfksyms +GENDWARFKSYMS_IN := $(GENDWARFKSYMS)-in.o + +LIBDW_FLAGS := $(shell $(HOSTPKG_CONFIG) libdw --cflags 2>/dev/null) +LIBDW_LIBS := $(shell $(HOSTPKG_CONFIG) libdw --libs 2>/dev/null || echo -ldw -lelf) + +all: $(GENDWARFKSYMS) + +INCLUDES := -I$(srctree)/tools/include + +WA
Re: [PATCH -fixes] riscv: Fix ftrace syscall handling which are now prefixed with __riscv_
On Tue, Oct 3, 2023 at 11:24 AM Alexandre Ghiti wrote:
>
> ftrace creates entries for each syscall in the tracefs but has failed
> since commit 08d0ce30e0e4 ("riscv: Implement syscall wrappers") which
> prefixes all riscv syscalls with __riscv_.
>
> So fix this by implementing arch_syscall_match_sym_name() which allows us
> to ignore this prefix.
>
> And also ignore compat syscalls like x86/arm64 by implementing
> arch_trace_is_compat_syscall().
>
> Fixes: 08d0ce30e0e4 ("riscv: Implement syscall wrappers")
> Signed-off-by: Alexandre Ghiti
> ---
> arch/riscv/include/asm/ftrace.h | 21 +
> 1 file changed, 21 insertions(+)
>
> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> index 740a979171e5..2b2f5df7ef2c 100644
> --- a/arch/riscv/include/asm/ftrace.h
> +++ b/arch/riscv/include/asm/ftrace.h
> @@ -31,6 +31,27 @@ static inline unsigned long ftrace_call_adjust(unsigned
> long addr)
> return addr;
> }
>
> +/*
> + * Let's do like x86/arm64 and ignore the compat syscalls.
> + */
> +#define ARCH_TRACE_IGNORE_COMPAT_SYSCALLS
> +static inline bool arch_trace_is_compat_syscall(struct pt_regs *regs)
> +{
> + return is_compat_task();
> +}
> +
> +#define ARCH_HAS_SYSCALL_MATCH_SYM_NAME
> +static inline bool arch_syscall_match_sym_name(const char *sym,
> + const char *name)
> +{
> + /*
> +* Since all syscall functions have __riscv_ prefix, we must skip it.
> +* However, as we described above, we decided to ignore compat
> +* syscalls, so we don't care about __riscv_compat_ prefix here.
> +*/
> + return !strcmp(sym + 8, name);
> +}
Good catch, thanks for fixing this!
Reviewed-by: Sami Tolvanen
Sami
Re: [PATCH 02/15] objtool: Add CONFIG_CFI_CLANG support
On Tue, Apr 20, 2021 at 12:48 PM Josh Poimboeuf wrote:
>
> On Fri, Apr 16, 2021 at 01:38:31PM -0700, Sami Tolvanen wrote:
> > +static int fix_cfi_relocs(const struct elf *elf)
> > +{
> > + struct section *sec, *tmpsec;
> > + struct reloc *reloc, *tmpreloc;
> > +
> > + list_for_each_entry_safe(sec, tmpsec, >sections, list) {
> > + list_for_each_entry_safe(reloc, tmpreloc, >reloc_list,
> > list) {
>
> These loops don't remove structs from the lists, so are the "safe"
> iterators really needed?
>
> > + struct symbol *sym;
> > + struct reloc *func_reloc;
> > +
> > + /*
> > + * CONFIG_CFI_CLANG replaces function relocations to
> > refer
> > + * to an intermediate jump table. Undo the conversion
> > so
> > + * objtool can make sense of things.
> > + */
>
> I think this comment could be clearer if it were placed above the
> function.
>
> > + if (!reloc->sym->sec->cfi_jt)
> > + continue;
> > +
> > + if (reloc->sym->type == STT_SECTION)
> > + sym = find_func_by_offset(reloc->sym->sec,
> > + reloc->addend);
> > + else
> > + sym = reloc->sym;
> > +
> > + if (!sym || !sym->sec)
> > + continue;
>
> This should be a fatal error, it probably means something's gone wrong
> with the reading of the jump table.
>
> > +
> > + /*
> > + * The jump table immediately jumps to the actual
> > function,
> > + * so look up the relocation there.
> > + */
> > + func_reloc = find_reloc_by_dest_range(elf, sym->sec,
> > sym->offset, 5);
>
> The jump instruction's reloc is always at offset 1, so this can maybe be
> optimized to:
>
> func_reloc = find_reloc_by_dest(elf, sym->sec,
> sym->offset+1);
>
> though of course that will probably break (future) arm64 objtool. Maybe
> an arch-specific offset from the start of the insn would be good.
>
> > + if (!func_reloc || !func_reloc->sym)
> > + continue;
>
> This should also be an error.
>
> > +
> > + reloc->sym = func_reloc->sym;
>
> I think we should also do 'reloc->addend = 0', because of the
> STT_SECTION case:
>
> 0025 259e000b R_X86_64_32S
> .text..L.cfi.jumptable.8047 + 8
>
> which then translates to
>
> 0009 06320004 R_X86_64_PLT32
> x2apic_prepare_cpu - 4
>
> so the original addend of '8' is no longer valid. Copying the '-4'
> wouldn't be right either, because that only applies to a PLT32 text
> reloc. A '0' should be appropriate for the 32S data reloc.
>
> This addend might not actually be read by any code, so it may not
> currently be an actual bug, but better safe than sorry.
Thank you for taking a look, Josh! I'll fix these in the next version.
Sami
Re: [PATCH 01/15] objtool: Find a destination for jumps beyond the section end
On Tue, Apr 20, 2021 at 11:14 AM Josh Poimboeuf wrote: > > On Fri, Apr 16, 2021 at 01:38:30PM -0700, Sami Tolvanen wrote: > > With -ffunction-sections, Clang can generate a jump beyond the end of > > a section when the section ends in an unreachable instruction. > > Why? Can you show an example? Here's the warning I'm seeing when building allyesconfig + CFI: vmlinux.o: warning: objtool: rockchip_spi_transfer_one.f088382d97b74759d70e27e891fe8f1c()+0x149: can't find jump dest instruction at .text.rockchip_spi_transfer_one.f088382d97b74759d70e27e891fe8f1c+0x7dc $ objdump -d -r -j .text.rockchip_spi_transfer_one.f088382d97b74759d70e27e891fe8f1c vmlinux.o : ... 149: 0f 85 8d 06 00 00 jne7dc <.compoundliteral.4> ... 7d7: e8 00 00 00 00 callq 7dc <.compoundliteral.4> 7d8: R_X86_64_PLT32 __stack_chk_fail-0x4 Sami
Re: [PATCH 09/15] x86/alternatives: Use C int3 selftest but disable KASAN
On Sat, Apr 17, 2021 at 4:37 AM Peter Zijlstra wrote:
>
> On Fri, Apr 16, 2021 at 01:38:38PM -0700, Sami Tolvanen wrote:
> > From: Kees Cook
> >
> > Instead of using inline asm for the int3 selftest (which confuses the
> > Clang's ThinLTO pass), this restores the C function but disables KASAN
> > (and tracing for good measure) to keep the things simple and avoid
> > unexpected side-effects. This attempts to keep the fix from commit
> > ecc606103837 ("x86/alternatives: Fix int3_emulate_call() selftest stack
> > corruption") without using inline asm.
> >
> > Signed-off-by: Kees Cook
> > Signed-off-by: Sami Tolvanen
> > ---
> > arch/x86/kernel/alternative.c | 21 -
> > 1 file changed, 4 insertions(+), 17 deletions(-)
> >
> > diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> > index 6974b5174495..669a23454c09 100644
> > --- a/arch/x86/kernel/alternative.c
> > +++ b/arch/x86/kernel/alternative.c
> > @@ -496,23 +496,10 @@ extern struct paravirt_patch_site
> > __start_parainstructions[],
> > *
> > * See entry_{32,64}.S for more details.
> > */
> > -
> > -/*
> > - * We define the int3_magic() function in assembly to control the calling
> > - * convention such that we can 'call' it from assembly.
> > - */
> > -
> > -extern void int3_magic(unsigned int *ptr); /* defined in asm */
> > -
> > -asm (
> > -".pushsection.init.text, \"ax\", @progbits\n"
> > -".type int3_magic, @function\n"
> > -"int3_magic:\n"
> > -"movl$1, (%" _ASM_ARG1 ")\n"
> > -"ret\n"
> > -".size int3_magic, .-int3_magic\n"
> > -".popsection\n"
> > -);
> > +static void __init __no_sanitize_address notrace int3_magic(unsigned int
> > *ptr)
> > +{
> > + *ptr = 1;
> > +}
>
> I really don't like this. the compiler is free to mess this up in all
> sorts of ways.
>
> The problem is that the call-site does not respect the regular calling
> convention, so calling a regular C function is just asking for trouble,
> which is why it ended up being asm, then we fully control the calling
> convention.
Ack. The problem here is that we can't declare an extern static
function in C. How would you feel about making int3_magic a global
instead to match the C declaration?
Sami
Re: [PATCH 05/15] x86: Implement function_nocfi
On Sun, Apr 18, 2021 at 3:57 PM Andy Lutomirski wrote: > > On Sun, Apr 18, 2021 at 9:17 AM Thomas Gleixner wrote: > > > > On Sat, Apr 17 2021 at 17:11, Andy Lutomirski wrote: > > > On Sat, Apr 17, 2021 at 4:53 PM Thomas Gleixner > > > wrote: > > >> which works for > > >> > > >> foo = function_nocfi(bar); > > > > > > I agree in general. But right now, we have, in asm/proto.h: > > > > > > void entry_SYSCALL_64(void); > > > > > > and that's pure nonsense. Depending on your point of view, > > > entry_SYSCALL_64 is a symbol that resolves to an integer or it's an > > > array of bytes containing instructions, but it is most definitely not > > > a function void (void). So, regardless of any CFI stuff, I propose > > > that we standardize our handling of prototypes of symbols that are > > > opaque to the C compiler. Here are a couple of choices: > > > > > > Easy one: > > > > > > extern u8 entry_SYSCALL_64[]; > > > > > > Slightly more complicated: > > > > > > struct opaque_symbol; > > > extern struct opaque_symbol entry_SYSCALL_64; > > > > > > The opaque_symbol variant avoids any possible confusion over the weird > > > status of arrays in C, and it's hard to misuse, since struct > > > opaque_symbol is an incomplete type. > > > > Makes sense. > > Sami, do you want to do this as part of your series or should I write a patch? I can certainly include this in the next version, but that might have to wait a bit for compiler changes, so if you want this done sooner, please go ahead. Sami
Re: [PATCH 05/15] x86: Implement function_nocfi
On Sat, Apr 17, 2021 at 3:16 AM Thomas Gleixner wrote: > > On Sat, Apr 17 2021 at 01:02, Thomas Gleixner wrote: > > On Fri, Apr 16 2021 at 15:37, Kees Cook wrote: > > > >> On Fri, Apr 16, 2021 at 03:20:17PM -0700, Andy Lutomirski wrote: > >>> But obviously there is code that needs real function pointers. How > >>> about making this a first-class feature, or at least hacking around it > >>> more cleanly. For example, what does this do: > >>> > >>> char entry_whatever[]; > >>> wrmsrl(..., (unsigned long)entry_whatever); > >> > >> This is just casting. It'll still resolve to the jump table entry. > >> > >>> or, alternatively, > >>> > >>> extern void func() __attribute__((nocfi)); > >> > >> __nocfi says func() should not perform checking of correct jump table > >> membership for indirect calls. > >> > >> But we don't want a global marking for a function to be ignored by CFI; > >> we don't want functions to escape CFI -- we want specific _users_ to > >> either not check CFI for indirect calls (__nocfi) or we want specific > >> passed addresses to avoid going through the jump table > >> (function_nocfi()). > > > > And that's why you mark entire files to be exempt without any rationale > > why it makes sense. > > The reason why you have to do that is because function_nocfi() is not > provided by the compiler. > > So you need to hack around that with that macro which fails to work > e.g. for the idt data arrays. > > Is there any fundamental reason why the compiler does not provide that > in a form which allows to use it everywhere? I'm not aware of a fundamental reason why the compiler couldn't provide a built-in here. This series attempts to work with what's available at the moment, and admittedly that's not quite ideal on x86. > It's not too much asked from a tool which provides new functionality to > provide it in a way which is usable. Sure, that's reasonable. I'll talk to our compiler folks and see how we can proceed here. Sami
Re: [PATCH 05/15] x86: Implement function_nocfi
On Fri, Apr 16, 2021 at 2:18 PM Borislav Petkov wrote:
>
> On Fri, Apr 16, 2021 at 01:38:34PM -0700, Sami Tolvanen wrote:
> > With CONFIG_CFI_CLANG, the compiler replaces function addresses in
> > instrumented C code with jump table addresses. This change implements
> > the function_nocfi() macro, which returns the actual function address
> > instead.
> >
> > Signed-off-by: Sami Tolvanen
> > ---
> > arch/x86/include/asm/page.h | 14 ++
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/arch/x86/include/asm/page.h b/arch/x86/include/asm/page.h
> > index 7555b48803a8..5499a05c44fc 100644
> > --- a/arch/x86/include/asm/page.h
> > +++ b/arch/x86/include/asm/page.h
> > @@ -71,6 +71,20 @@ static inline void copy_user_page(void *to, void *from,
> > unsigned long vaddr,
> > extern bool __virt_addr_valid(unsigned long kaddr);
> > #define virt_addr_valid(kaddr) __virt_addr_valid((unsigned long)
> > (kaddr))
> >
> > +#ifdef CONFIG_CFI_CLANG
>
> Almost every patch is talking about this magical config symbol but it
> is nowhere to be found. How do I disable it, is there a Kconfig entry
> somewhere, etc, etc?
As I mentioned in the cover letter, this series is based on
linux-next. I forgot to include a link to the original patch series
that adds CONFIG_CFI_CLANG, but I'll be sure to point to it in the
next version. Sorry about the confusion.
> > +/*
> > + * With CONFIG_CFI_CLANG, the compiler replaces function address
> > + * references with the address of the function's CFI jump table
> > + * entry. The function_nocfi macro always returns the address of the
> > + * actual function instead.
> > + */
> > +#define function_nocfi(x) ({ \
> > + void *addr; \
> > + asm("leaq " __stringify(x) "(%%rip), %0\n\t" : "=r" (addr));\
> > + addr; \
> > +})
>
> Also, eww.
>
> It seems all these newfangled things we get, mean obfuscating code even
> more. What's wrong with using __nocfi instead? That's nicely out of the
> way so slap that in front of functions.
__nocfi only disables CFI checking in a function, the compiler still
changes function addresses to point to the CFI jump table, which is
why we need function_nocfi().
> Also, did you even build this on, say, 32-bit allmodconfig?
>
> Oh well.
>
> In file included from ./include/linux/ftrace.h:22:0,
> from ./include/linux/init_task.h:9,
> from init/init_task.c:2:
> ./include/linux/ftrace.h: In function ‘ftrace_init_nop’:
> ./arch/x86/include/asm/ftrace.h:9:40: error: implicit declaration of function
> ‘function_nocfi’ [-Werror=implicit-function-declaration]
This is defined in linux-next, but I do see another issue, which I'll
fix in v2. Note that CFI_CLANG itself cannot be selected on 32-bit
x86.
Sami
[PATCH 15/15] x86, build: Allow CONFIG_CFI_CLANG to be selected
Select ARCH_SUPPORTS_CFI_CLANG to allow CFI to be enabled. Signed-off-by: Sami Tolvanen --- arch/x86/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index bf69d07e46b8..81d2dc568e56 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -106,6 +106,7 @@ config X86 select ARCH_SUPPORTS_KMAP_LOCAL_FORCE_MAP if NR_CPUS <= 4096 select ARCH_SUPPORTS_LTO_CLANG if X86_64 select ARCH_SUPPORTS_LTO_CLANG_THIN if X86_64 + select ARCH_SUPPORTS_CFI_CLANG if X86_64 select ARCH_USE_BUILTIN_BSWAP select ARCH_USE_MEMTEST select ARCH_USE_QUEUED_RWLOCKS -- 2.31.1.368.gbe11c130af-goog
[PATCH 14/15] x86, kprobes: Fix optprobe_template_func type mismatch
The optprobe_template_func symbol is defined in inline assembly, but it's not marked global, which conflicts with the C declaration needed for STACK_FRAME_NON_STANDARD and confuses the compiler when CONFIG_CFI_CLANG is enabled. Marking the symbol global would make the compiler happy, but as the compiler also generates a CFI jump table entry for all address-taken functions, the jump table ends up containing a jump to the .rodata section where optprobe_template_func resides, which results in an objtool warning. Use ASM_STACK_FRAME_NON_STANDARD instead to avoid both issues. Signed-off-by: Sami Tolvanen --- arch/x86/kernel/kprobes/opt.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c index 71425ebba98a..95375ef5deee 100644 --- a/arch/x86/kernel/kprobes/opt.c +++ b/arch/x86/kernel/kprobes/opt.c @@ -103,6 +103,7 @@ static void synthesize_set_arg1(kprobe_opcode_t *addr, unsigned long val) asm ( ".pushsection .rodata\n" "optprobe_template_func:\n" + ASM_STACK_FRAME_NON_STANDARD(optprobe_template_func) ".global optprobe_template_entry\n" "optprobe_template_entry:\n" #ifdef CONFIG_X86_64 @@ -154,9 +155,6 @@ asm ( "optprobe_template_end:\n" ".popsection\n"); -void optprobe_template_func(void); -STACK_FRAME_NON_STANDARD(optprobe_template_func); - #define TMPL_CLAC_IDX \ ((long)optprobe_template_clac - (long)optprobe_template_entry) #define TMPL_MOVE_IDX \ -- 2.31.1.368.gbe11c130af-goog
[PATCH 12/15] x86, module: Ignore __typeid__ relocations
Ignore the __typeid__ relocations generated with CONFIG_CFI_CLANG
when loading modules.
Signed-off-by: Sami Tolvanen
---
arch/x86/kernel/module.c | 4
1 file changed, 4 insertions(+)
diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
index 5e9a34b5bd74..c4aeba237eef 100644
--- a/arch/x86/kernel/module.c
+++ b/arch/x86/kernel/module.c
@@ -197,6 +197,10 @@ static int __apply_relocate_add(Elf64_Shdr *sechdrs,
val -= (u64)loc;
write(loc, , 8);
break;
+ case R_X86_64_8:
+ if (!strncmp(strtab + sym->st_name, "__typeid__", 10))
+ break;
+ fallthrough;
default:
pr_err("%s: Unknown rela relocation: %llu\n",
me->name, ELF64_R_TYPE(rel[i].r_info));
--
2.31.1.368.gbe11c130af-goog
[PATCH 13/15] x86, cpu: Use LTO for cpu.c with CFI
Allow LTO to be used for cpu.c when CONFIG_CFI_CLANG is enabled to avoid indirect call failures. CFI requires Clang >= 12, which doesn't have the stack protector inlining bug. Signed-off-by: Sami Tolvanen --- arch/x86/power/Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/x86/power/Makefile b/arch/x86/power/Makefile index 379777572bc9..a0532851fed7 100644 --- a/arch/x86/power/Makefile +++ b/arch/x86/power/Makefile @@ -4,9 +4,11 @@ # itself be stack-protected CFLAGS_cpu.o := -fno-stack-protector +ifndef CONFIG_CFI_CLANG # Clang may incorrectly inline functions with stack protector enabled into # __restore_processor_state(): https://bugs.llvm.org/show_bug.cgi?id=47479 CFLAGS_REMOVE_cpu.o := $(CC_FLAGS_LTO) +endif obj-$(CONFIG_PM_SLEEP) += cpu.o obj-$(CONFIG_HIBERNATION) += hibernate_$(BITS).o hibernate_asm_$(BITS).o hibernate.o -- 2.31.1.368.gbe11c130af-goog
[PATCH 11/15] x86, relocs: Ignore __typeid__ relocations
From: Kees Cook
The __typeid__* symbols aren't actually relocations, so they can be
ignored during relocation generation.
Signed-off-by: Kees Cook
Signed-off-by: Sami Tolvanen
---
arch/x86/tools/relocs.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index 04c5a44b9682..78516ccea0c8 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -48,6 +48,7 @@ static const char * const sym_regex_kernel[S_NSYMTYPES] = {
"^(xen_irq_disable_direct_reloc$|"
"xen_save_fl_direct_reloc$|"
"VDSO|"
+ "__typeid__|"
"__crc_)",
/*
@@ -808,6 +809,12 @@ static int do_reloc64(struct section *sec, Elf_Rel *rel,
ElfW(Sym) *sym,
symname);
break;
+ case R_X86_64_8:
+ if (!shn_abs || !is_reloc(S_ABS, symname))
+ die("Non-whitelisted %s relocation: %s\n",
+ rel_type(r_type), symname);
+ break;
+
case R_X86_64_32:
case R_X86_64_32S:
case R_X86_64_64:
--
2.31.1.368.gbe11c130af-goog
[PATCH 10/15] x86/purgatory: Disable CFI
Disable CONFIG_CFI_CLANG for the stand-alone purgatory.ro. Signed-off-by: Sami Tolvanen --- arch/x86/purgatory/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile index 95ea17a9d20c..ed46ad780130 100644 --- a/arch/x86/purgatory/Makefile +++ b/arch/x86/purgatory/Makefile @@ -31,7 +31,7 @@ KCOV_INSTRUMENT := n # These are adjustments to the compiler flags used for objects that # make up the standalone purgatory.ro -PURGATORY_CFLAGS_REMOVE := -mcmodel=kernel +PURGATORY_CFLAGS_REMOVE := -mcmodel=kernel $(CC_FLAGS_CFI) PURGATORY_CFLAGS := -mcmodel=large -ffreestanding -fno-zero-initialized-in-bss -g0 PURGATORY_CFLAGS += $(DISABLE_STACKLEAK_PLUGIN) -DDISABLE_BRANCH_PROFILING PURGATORY_CFLAGS += -fno-stack-protector -- 2.31.1.368.gbe11c130af-goog
[PATCH 08/15] x86/extable: Do not mark exception callback as CFI
From: Kees Cook
The exception table entries are constructed out of a relative offset
and point to the actual function, not the CFI table entry. For now,
just mark the caller as not checking CFI. The failure is most visible
at boot with CONFIG_DEBUG_RODATA_TEST=y.
Signed-off-by: Kees Cook
Signed-off-by: Sami Tolvanen
---
arch/x86/mm/extable.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index b93d6cd08a7f..a7eae1c4c59f 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -155,6 +155,7 @@ enum handler_type ex_get_fault_handler_type(unsigned long
ip)
return EX_HANDLER_OTHER;
}
+__nocfi
int fixup_exception(struct pt_regs *regs, int trapnr, unsigned long error_code,
unsigned long fault_addr)
{
--
2.31.1.368.gbe11c130af-goog
[PATCH 09/15] x86/alternatives: Use C int3 selftest but disable KASAN
From: Kees Cook
Instead of using inline asm for the int3 selftest (which confuses the
Clang's ThinLTO pass), this restores the C function but disables KASAN
(and tracing for good measure) to keep the things simple and avoid
unexpected side-effects. This attempts to keep the fix from commit
ecc606103837 ("x86/alternatives: Fix int3_emulate_call() selftest stack
corruption") without using inline asm.
Signed-off-by: Kees Cook
Signed-off-by: Sami Tolvanen
---
arch/x86/kernel/alternative.c | 21 -
1 file changed, 4 insertions(+), 17 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 6974b5174495..669a23454c09 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -496,23 +496,10 @@ extern struct paravirt_patch_site
__start_parainstructions[],
*
* See entry_{32,64}.S for more details.
*/
-
-/*
- * We define the int3_magic() function in assembly to control the calling
- * convention such that we can 'call' it from assembly.
- */
-
-extern void int3_magic(unsigned int *ptr); /* defined in asm */
-
-asm (
-" .pushsection.init.text, \"ax\", @progbits\n"
-" .type int3_magic, @function\n"
-"int3_magic:\n"
-" movl$1, (%" _ASM_ARG1 ")\n"
-" ret\n"
-" .size int3_magic, .-int3_magic\n"
-" .popsection\n"
-);
+static void __init __no_sanitize_address notrace int3_magic(unsigned int *ptr)
+{
+ *ptr = 1;
+}
extern __initdata unsigned long int3_selftest_ip; /* defined in asm below */
--
2.31.1.368.gbe11c130af-goog
[PATCH 04/15] static_call: Use global functions for the self-test
With CONFIG_CFI_CLANG, the compiler renames static functions. This
breaks static_call users using static functions, because the current
implementation assumes functions have stable names by hardcoding them
in inline assembly. Make the self-test functions global to prevent the
compiler from renaming them.
Signed-off-by: Sami Tolvanen
---
kernel/static_call.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/static_call.c b/kernel/static_call.c
index 723fcc9d20db..d09f500c2d2a 100644
--- a/kernel/static_call.c
+++ b/kernel/static_call.c
@@ -503,12 +503,12 @@ long __static_call_return0(void)
#ifdef CONFIG_STATIC_CALL_SELFTEST
-static int func_a(int x)
+int func_a(int x)
{
return x+1;
}
-static int func_b(int x)
+int func_b(int x)
{
return x+2;
}
--
2.31.1.368.gbe11c130af-goog
[PATCH 03/15] objtool: Add ASM_STACK_FRAME_NON_STANDARD
To use the STACK_FRAME_NON_STANDARD macro for a static symbol
defined in inline assembly, we need a C declaration that implies
global visibility. This type mismatch confuses the compiler with
CONFIG_CFI_CLANG. This change adds an inline assembly version of
the macro to avoid the issue.
Signed-off-by: Sami Tolvanen
---
include/linux/objtool.h | 5 +
tools/include/linux/objtool.h | 5 +
2 files changed, 10 insertions(+)
diff --git a/include/linux/objtool.h b/include/linux/objtool.h
index 7e72d975cb76..2f29ce48ab5f 100644
--- a/include/linux/objtool.h
+++ b/include/linux/objtool.h
@@ -66,6 +66,11 @@ struct unwind_hint {
static void __used __section(".discard.func_stack_frame_non_standard") \
*__func_stack_frame_non_standard_##func = func
+#define ASM_STACK_FRAME_NON_STANDARD(func) \
+ ".pushsection .discard.func_stack_frame_non_standard, \"aw\"\n" \
+ ".long " __stringify(func) " - .\n" \
+ ".popsection\n"
+
#else /* __ASSEMBLY__ */
/*
diff --git a/tools/include/linux/objtool.h b/tools/include/linux/objtool.h
index 7e72d975cb76..2f29ce48ab5f 100644
--- a/tools/include/linux/objtool.h
+++ b/tools/include/linux/objtool.h
@@ -66,6 +66,11 @@ struct unwind_hint {
static void __used __section(".discard.func_stack_frame_non_standard") \
*__func_stack_frame_non_standard_##func = func
+#define ASM_STACK_FRAME_NON_STANDARD(func) \
+ ".pushsection .discard.func_stack_frame_non_standard, \"aw\"\n" \
+ ".long " __stringify(func) " - .\n" \
+ ".popsection\n"
+
#else /* __ASSEMBLY__ */
/*
--
2.31.1.368.gbe11c130af-goog
[PATCH 07/15] x86/ftrace: Use function_nocfi in MCOUNT_ADDR
With CONFIG_CFI_CLANG, the compiler replaces the __fentry__ address in MCOUNT_ADDR with the address of a CFI jump table. Use function_nocfi() to get the actual function address. Signed-off-by: Sami Tolvanen --- arch/x86/include/asm/ftrace.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h index 9f3130f40807..0b7567994f4a 100644 --- a/arch/x86/include/asm/ftrace.h +++ b/arch/x86/include/asm/ftrace.h @@ -6,7 +6,7 @@ #ifndef CC_USING_FENTRY # error Compiler does not support fentry? #endif -# define MCOUNT_ADDR ((unsigned long)(__fentry__)) +# define MCOUNT_ADDR ((unsigned long)(function_nocfi(__fentry__))) #define MCOUNT_INSN_SIZE 5 /* sizeof mcount call */ #ifdef CONFIG_DYNAMIC_FTRACE -- 2.31.1.368.gbe11c130af-goog
[PATCH 06/15] x86: Avoid CFI jump tables in IDT and entry points
With CONFIG_CFI_CLANG, the compiler replaces function addresses in C
code with jump table addresses. To avoid referring to jump tables in
entry code with PTI, disable CFI for IDT and paravirt code, and use
function_nocfi() to prevent jump table addresses from being added to
the IDT or system call entry points.
Reported-by: Sedat Dilek
Signed-off-by: Sami Tolvanen
Tested-by: Sedat Dilek
---
arch/x86/include/asm/desc.h | 8 +++-
arch/x86/kernel/Makefile | 3 +++
arch/x86/kernel/cpu/common.c | 8
arch/x86/kernel/idt.c| 2 +-
arch/x86/kernel/traps.c | 2 +-
arch/x86/xen/Makefile| 2 ++
6 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h
index 476082a83d1c..9256eec2 100644
--- a/arch/x86/include/asm/desc.h
+++ b/arch/x86/include/asm/desc.h
@@ -381,7 +381,13 @@ static inline void set_desc_limit(struct desc_struct
*desc, unsigned long limit)
desc->limit1 = (limit >> 16) & 0xf;
}
-void alloc_intr_gate(unsigned int n, const void *addr);
+/*
+ * Use function_nocfi() to prevent the compiler from replacing function
+ * addresses with jump table addresses when CONFIG_CFI_CLANG is enabled.
+ */
+#define alloc_intr_gate(n, addr) __alloc_intr_gate(n, function_nocfi(addr))
+
+void __alloc_intr_gate(unsigned int n, const void *addr);
static inline void init_idt_data(struct idt_data *data, unsigned int n,
const void *addr)
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 0704c2a94272..12a739eb208e 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -46,6 +46,9 @@ endif
# non-deterministic coverage.
KCOV_INSTRUMENT:= n
+CFLAGS_REMOVE_idt.o:= $(CC_FLAGS_CFI)
+CFLAGS_REMOVE_paravirt.o := $(CC_FLAGS_CFI)
+
CFLAGS_head$(BITS).o += -fno-stack-protector
CFLAGS_irq.o := -I $(srctree)/$(src)/../include/asm/trace
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 6bdb69a9a7dc..e6cff98b182a 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1752,10 +1752,10 @@ DEFINE_PER_CPU(unsigned long, cpu_current_top_of_stack)
= TOP_OF_INIT_STACK;
void syscall_init(void)
{
wrmsr(MSR_STAR, 0, (__USER32_CS << 16) | __KERNEL_CS);
- wrmsrl(MSR_LSTAR, (unsigned long)entry_SYSCALL_64);
+ wrmsrl(MSR_LSTAR, (unsigned long)function_nocfi(entry_SYSCALL_64));
#ifdef CONFIG_IA32_EMULATION
- wrmsrl(MSR_CSTAR, (unsigned long)entry_SYSCALL_compat);
+ wrmsrl(MSR_CSTAR, (unsigned long)function_nocfi(entry_SYSCALL_compat));
/*
* This only works on Intel CPUs.
* On AMD CPUs these MSRs are 32-bit, CPU truncates
MSR_IA32_SYSENTER_EIP.
@@ -1765,9 +1765,9 @@ void syscall_init(void)
wrmsrl_safe(MSR_IA32_SYSENTER_CS, (u64)__KERNEL_CS);
wrmsrl_safe(MSR_IA32_SYSENTER_ESP,
(unsigned long)(cpu_entry_stack(smp_processor_id()) + 1));
- wrmsrl_safe(MSR_IA32_SYSENTER_EIP, (u64)entry_SYSENTER_compat);
+ wrmsrl_safe(MSR_IA32_SYSENTER_EIP,
(u64)function_nocfi(entry_SYSENTER_compat));
#else
- wrmsrl(MSR_CSTAR, (unsigned long)ignore_sysret);
+ wrmsrl(MSR_CSTAR, (unsigned long)function_nocfi(ignore_sysret));
wrmsrl_safe(MSR_IA32_SYSENTER_CS, (u64)GDT_ENTRY_INVALID_SEG);
wrmsrl_safe(MSR_IA32_SYSENTER_ESP, 0ULL);
wrmsrl_safe(MSR_IA32_SYSENTER_EIP, 0ULL);
diff --git a/arch/x86/kernel/idt.c b/arch/x86/kernel/idt.c
index d552f177eca0..6574a742e373 100644
--- a/arch/x86/kernel/idt.c
+++ b/arch/x86/kernel/idt.c
@@ -340,7 +340,7 @@ void idt_invalidate(void *addr)
load_idt();
}
-void __init alloc_intr_gate(unsigned int n, const void *addr)
+void __init __alloc_intr_gate(unsigned int n, const void *addr)
{
if (WARN_ON(n < FIRST_SYSTEM_VECTOR))
return;
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 853ea7a80806..54616dc49116 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -403,7 +403,7 @@ DEFINE_IDTENTRY_DF(exc_double_fault)
* which is what the stub expects, given that the faulting
* RIP will be the IRET instruction.
*/
- regs->ip = (unsigned long)asm_exc_general_protection;
+ regs->ip = (unsigned
long)function_nocfi(asm_exc_general_protection);
regs->sp = (unsigned long)>orig_ax;
return;
diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
index 40b5779fce21..61e2d9b2fa43 100644
--- a/arch/x86/xen/Makefile
+++ b/arch/x86/xen/Makefile
@@ -11,6 +11,8 @@ endif
CFLAGS_enlighten_pv.o := -fno-stack-protector
CFLAGS_mmu_pv.o:= -fno-stack-protector
+CFLAGS_REMOVE_enlighten_pv.o := $(CC_FLAGS_CFI)
+
obj-y += enlighten.o
obj-y
[PATCH 05/15] x86: Implement function_nocfi
With CONFIG_CFI_CLANG, the compiler replaces function addresses in
instrumented C code with jump table addresses. This change implements
the function_nocfi() macro, which returns the actual function address
instead.
Signed-off-by: Sami Tolvanen
---
arch/x86/include/asm/page.h | 14 ++
1 file changed, 14 insertions(+)
diff --git a/arch/x86/include/asm/page.h b/arch/x86/include/asm/page.h
index 7555b48803a8..5499a05c44fc 100644
--- a/arch/x86/include/asm/page.h
+++ b/arch/x86/include/asm/page.h
@@ -71,6 +71,20 @@ static inline void copy_user_page(void *to, void *from,
unsigned long vaddr,
extern bool __virt_addr_valid(unsigned long kaddr);
#define virt_addr_valid(kaddr) __virt_addr_valid((unsigned long) (kaddr))
+#ifdef CONFIG_CFI_CLANG
+/*
+ * With CONFIG_CFI_CLANG, the compiler replaces function address
+ * references with the address of the function's CFI jump table
+ * entry. The function_nocfi macro always returns the address of the
+ * actual function instead.
+ */
+#define function_nocfi(x) ({ \
+ void *addr; \
+ asm("leaq " __stringify(x) "(%%rip), %0\n\t" : "=r" (addr));\
+ addr; \
+})
+#endif
+
#endif /* __ASSEMBLY__ */
#include
--
2.31.1.368.gbe11c130af-goog
[PATCH 02/15] objtool: Add CONFIG_CFI_CLANG support
With CONFIG_CFI_CLANG, the compiler replaces function references with
references to the CFI jump table, which confuses objtool. This change,
based on Josh's initial patch [1], goes through the list of relocations
and replaces jump table symbols with the actual function symbols.
[1]
https://lore.kernel.org/r/d743f4b36e120c06506567a9f87a062ae03da47f.1611263462.git.jpoim...@redhat.com/
Reported-by: Sedat Dilek
Suggested-by: Josh Poimboeuf
Signed-off-by: Sami Tolvanen
---
tools/objtool/elf.c | 48 +
tools/objtool/include/objtool/elf.h | 2 +-
2 files changed, 49 insertions(+), 1 deletion(-)
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index d08f5f3670f8..5cf2c61ce7b8 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -273,6 +273,10 @@ static int read_sections(struct elf *elf)
}
sec->len = sec->sh.sh_size;
+ /* Detect -fsanitize=cfi jump table sections */
+ if (!strncmp(sec->name, ".text..L.cfi.jumptable", 22))
+ sec->cfi_jt = true;
+
list_add_tail(>list, >sections);
elf_hash_add(elf->section_hash, >hash, sec->idx);
elf_hash_add(elf->section_name_hash, >name_hash,
str_hash(sec->name));
@@ -548,6 +552,48 @@ static int read_rela_reloc(struct section *sec, int i,
struct reloc *reloc, unsi
return 0;
}
+static int fix_cfi_relocs(const struct elf *elf)
+{
+ struct section *sec, *tmpsec;
+ struct reloc *reloc, *tmpreloc;
+
+ list_for_each_entry_safe(sec, tmpsec, >sections, list) {
+ list_for_each_entry_safe(reloc, tmpreloc, >reloc_list,
list) {
+ struct symbol *sym;
+ struct reloc *func_reloc;
+
+ /*
+* CONFIG_CFI_CLANG replaces function relocations to
refer
+* to an intermediate jump table. Undo the conversion so
+* objtool can make sense of things.
+*/
+ if (!reloc->sym->sec->cfi_jt)
+ continue;
+
+ if (reloc->sym->type == STT_SECTION)
+ sym = find_func_by_offset(reloc->sym->sec,
+ reloc->addend);
+ else
+ sym = reloc->sym;
+
+ if (!sym || !sym->sec)
+ continue;
+
+ /*
+* The jump table immediately jumps to the actual
function,
+* so look up the relocation there.
+*/
+ func_reloc = find_reloc_by_dest_range(elf, sym->sec,
sym->offset, 5);
+ if (!func_reloc || !func_reloc->sym)
+ continue;
+
+ reloc->sym = func_reloc->sym;
+ }
+ }
+
+ return 0;
+}
+
static int read_relocs(struct elf *elf)
{
struct section *sec;
@@ -608,6 +654,8 @@ static int read_relocs(struct elf *elf)
tot_reloc += nr_reloc;
}
+ fix_cfi_relocs(elf);
+
if (stats) {
printf("max_reloc: %lu\n", max_reloc);
printf("tot_reloc: %lu\n", tot_reloc);
diff --git a/tools/objtool/include/objtool/elf.h
b/tools/objtool/include/objtool/elf.h
index 45e5ede363b0..ef19578fc5e4 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -39,7 +39,7 @@ struct section {
char *name;
int idx;
unsigned int len;
- bool changed, text, rodata, noinstr;
+ bool changed, text, rodata, noinstr, cfi_jt;
};
struct symbol {
--
2.31.1.368.gbe11c130af-goog
[PATCH 01/15] objtool: Find a destination for jumps beyond the section end
With -ffunction-sections, Clang can generate a jump beyond the end of
a section when the section ends in an unreachable instruction. If the
offset matches the section length, use the last instruction as the
jump destination.
Signed-off-by: Sami Tolvanen
---
tools/objtool/check.c | 4
1 file changed, 4 insertions(+)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index a0a8ce61cd4d..5fd60e055a5c 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -870,6 +870,10 @@ static int add_jump_destinations(struct objtool_file *file)
}
insn->jump_dest = find_insn(file, dest_sec, dest_off);
+
+ if (!insn->jump_dest && dest_sec->len == dest_off)
+ insn->jump_dest = find_last_insn(file, dest_sec);
+
if (!insn->jump_dest) {
/*
--
2.31.1.368.gbe11c130af-goog
[PATCH 00/15] x86: Add support for Clang CFI
This series adds support for Clang's Control-Flow Integrity (CFI) checking for x86_64. With CFI, the compiler injects a runtime check before each indirect function call to ensure the target is a valid function with the correct static type. This restricts possible call targets and makes it more difficult for an attacker to exploit bugs that allow the modification of stored function pointers. For more details, see: https://clang.llvm.org/docs/ControlFlowIntegrity.html The first two patches contain objtool support for CFI, and the remaining patches disable CFI where it shouldn't be used and fix other smaller issues, such as type conflicts that confuse the compiler. Note that the patches are based on next-20210416. You can also pull the series from https://github.com/samitolvanen/linux.git x86-cfi-v1 Kees Cook (3): x86/extable: Do not mark exception callback as CFI x86/alternatives: Use C int3 selftest but disable KASAN x86, relocs: Ignore __typeid__ relocations Sami Tolvanen (12): objtool: Find a destination for jumps beyond the section end objtool: Add CONFIG_CFI_CLANG support objtool: Add ASM_STACK_FRAME_NON_STANDARD static_call: Use global functions for the self-test x86: Implement function_nocfi x86: Avoid CFI jump tables in IDT and entry points x86/ftrace: Use function_nocfi in MCOUNT_ADDR x86/purgatory: Disable CFI x86, module: Ignore __typeid__ relocations x86, cpu: Use LTO for cpu.c with CFI x86, kprobes: Fix optprobe_template_func type mismatch x86, build: Allow CONFIG_CFI_CLANG to be selected arch/x86/Kconfig| 1 + arch/x86/include/asm/desc.h | 8 - arch/x86/include/asm/ftrace.h | 2 +- arch/x86/include/asm/page.h | 14 + arch/x86/kernel/Makefile| 3 ++ arch/x86/kernel/alternative.c | 21 +++-- arch/x86/kernel/cpu/common.c| 8 ++--- arch/x86/kernel/idt.c | 2 +- arch/x86/kernel/kprobes/opt.c | 4 +-- arch/x86/kernel/module.c| 4 +++ arch/x86/kernel/traps.c | 2 +- arch/x86/mm/extable.c | 1 + arch/x86/power/Makefile | 2 ++ arch/x86/purgatory/Makefile | 2 +- arch/x86/tools/relocs.c | 7 + arch/x86/xen/Makefile | 2 ++ include/linux/objtool.h | 5 +++ kernel/static_call.c| 4 +-- tools/include/linux/objtool.h | 5 +++ tools/objtool/check.c | 4 +++ tools/objtool/elf.c | 48 + tools/objtool/include/objtool/elf.h | 2 +- 22 files changed, 119 insertions(+), 32 deletions(-) base-commit: 18250b538735142307082e4e99e3ae5c12d44013 -- 2.31.1.368.gbe11c130af-goog
Re: [PATCH] arm64: alternatives: Move length validation in alternative_{insn,endif}
On Thu, Apr 15, 2021 at 7:02 AM Catalin Marinas wrote:
>
> On Thu, Apr 15, 2021 at 06:25:57AM -0700, Nathan Chancellor wrote:
> > On Thu, Apr 15, 2021 at 10:17:43AM +0100, Catalin Marinas wrote:
> > > On Tue, Apr 13, 2021 at 05:08:04PM -0700, Nathan Chancellor wrote:
> > > > After commit 2decad92f473 ("arm64: mte: Ensure TIF_MTE_ASYNC_FAULT is
> > > > set atomically"), LLVM's integrated assembler fails to build entry.S:
> > > >
> > > > :5:7: error: expected assembly-time absolute expression
> > > > .org . - (664b-663b) + (662b-661b)
> > > > ^
> > > > :6:7: error: expected assembly-time absolute expression
> > > > .org . - (662b-661b) + (664b-663b)
> > > > ^
> > >
> > > I tried the latest Linus' tree and linux-next (defconfig) with this
> > > commit in and I can't get your build error. I used both clang-10 from
> > > Debian stable and clang-11 from Debian sid. So, which clang version did
> > > you use or which kernel config options?
> >
> > Interesting, this reproduces for me with LLVM 12 or newer with just
> > defconfig.
>
> It fails for me as well with clang-12. Do you happen to know why it
> works fine with previous clang versions?
It looks like CONFIG_ARM64_AS_HAS_MTE is not set when we use the
integrated assembler with LLVM 11, and the code that breaks later
versions is gated behind CONFIG_ARM64_MTE.
Sami
Re: [PATCH] arm64: alternatives: Move length validation in alternative_{insn,endif}
Hi Nathan,
On Tue, Apr 13, 2021 at 5:09 PM Nathan Chancellor wrote:
>
> After commit 2decad92f473 ("arm64: mte: Ensure TIF_MTE_ASYNC_FAULT is
> set atomically"), LLVM's integrated assembler fails to build entry.S:
>
> :5:7: error: expected assembly-time absolute expression
> .org . - (664b-663b) + (662b-661b)
> ^
> :6:7: error: expected assembly-time absolute expression
> .org . - (662b-661b) + (664b-663b)
> ^
>
> The root cause is LLVM's assembler has a one-pass design, meaning it
> cannot figure out these instruction lengths when the .org directive is
> outside of the subsection that they are in, which was changed by the
> .arch_extension directive added in the above commit.
>
> Apply the same fix from commit 966a0acce2fc ("arm64/alternatives: move
> length validation inside the subsection") to the alternative_endif
> macro, shuffling the .org directives so that the length validation
> happen will always happen in the same subsections. alternative_insn has
> not shown any issue yet but it appears that it could have the same issue
> in the future so just preemptively change it.
>
> Cc: sta...@vger.kernel.org
> Fixes: f7b93d42945c ("arm64/alternatives: use subsections for replacement
> sequences")
> Link: https://github.com/ClangBuiltLinux/linux/issues/1347
> Signed-off-by: Nathan Chancellor
> ---
>
> Apologies if my explanation or terminology is off, I am only now getting
> more familiar with assembly.
>
> arch/arm64/include/asm/alternative-macros.h | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/include/asm/alternative-macros.h
> b/arch/arm64/include/asm/alternative-macros.h
> index 5df500dcc627..8a078fc662ac 100644
> --- a/arch/arm64/include/asm/alternative-macros.h
> +++ b/arch/arm64/include/asm/alternative-macros.h
> @@ -97,9 +97,9 @@
> .popsection
> .subsection 1
> 663: \insn2
> -664: .previous
> - .org. - (664b-663b) + (662b-661b)
> +664: .org. - (664b-663b) + (662b-661b)
> .org. - (662b-661b) + (664b-663b)
> + .previous
> .endif
> .endm
>
> @@ -169,11 +169,11 @@
> */
> .macro alternative_endif
> 664:
> + .org. - (664b-663b) + (662b-661b)
> + .org. - (662b-661b) + (664b-663b)
> .if .Lasm_alt_mode==0
> .previous
> .endif
> - .org. - (664b-663b) + (662b-661b)
> - .org. - (662b-661b) + (664b-663b)
> .endm
>
> /*
Thank you for fixing these!
The patch looks correct to me, next-20210413 builds with LLVM_IAS=1
after I applied it, and defconfig built with both Clang and gcc boots
normally. Please feel free to add:
Reviewed-by: Sami Tolvanen
Tested-by: Sami Tolvanen
Sami
[PATCH v6 17/18] KVM: arm64: Disable CFI for nVHE
Disable CFI for the nVHE code to avoid address space confusion. Signed-off-by: Sami Tolvanen Reviewed-by: Kees Cook Tested-by: Nathan Chancellor --- arch/arm64/kvm/hyp/nvhe/Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile index a6707df4f6c0..fb24a0f022ad 100644 --- a/arch/arm64/kvm/hyp/nvhe/Makefile +++ b/arch/arm64/kvm/hyp/nvhe/Makefile @@ -75,9 +75,9 @@ quiet_cmd_hyprel = HYPREL $@ quiet_cmd_hypcopy = HYPCOPY $@ cmd_hypcopy = $(OBJCOPY) --prefix-symbols=__kvm_nvhe_ $< $@ -# Remove ftrace and Shadow Call Stack CFLAGS. -# This is equivalent to the 'notrace' and '__noscs' annotations. -KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS), $(KBUILD_CFLAGS)) +# Remove ftrace, Shadow Call Stack, and CFI CFLAGS. +# This is equivalent to the 'notrace', '__noscs', and '__nocfi' annotations. +KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS) $(CC_FLAGS_CFI), $(KBUILD_CFLAGS)) # KVM nVHE code is run at a different exception code with a different map, so # compiler instrumentation that inserts callbacks or checks into the code may -- 2.31.1.295.g9ea45b61b8-goog
[PATCH v6 18/18] arm64: allow CONFIG_CFI_CLANG to be selected
Select ARCH_SUPPORTS_CFI_CLANG to allow CFI to be enabled. Signed-off-by: Sami Tolvanen Reviewed-by: Kees Cook Tested-by: Nathan Chancellor --- arch/arm64/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index e4e1b6550115..d7395772b6b8 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -75,6 +75,7 @@ config ARM64 select ARCH_SUPPORTS_SHADOW_CALL_STACK if CC_HAVE_SHADOW_CALL_STACK select ARCH_SUPPORTS_LTO_CLANG if CPU_LITTLE_ENDIAN select ARCH_SUPPORTS_LTO_CLANG_THIN + select ARCH_SUPPORTS_CFI_CLANG select ARCH_SUPPORTS_ATOMIC_RMW select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && (GCC_VERSION >= 5 || CC_IS_CLANG) select ARCH_SUPPORTS_NUMA_BALANCING -- 2.31.1.295.g9ea45b61b8-goog
[PATCH v6 15/18] arm64: add __nocfi to __apply_alternatives
__apply_alternatives makes indirect calls to functions whose address
is taken in assembly code using the alternative_cb macro. With
non-canonical CFI, the compiler won't replace these function
references with the jump table addresses, which trips CFI. Disable CFI
checking in the function to work around the issue.
Signed-off-by: Sami Tolvanen
Reviewed-by: Kees Cook
Tested-by: Nathan Chancellor
---
arch/arm64/kernel/alternative.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/alternative.c b/arch/arm64/kernel/alternative.c
index 1184c44ea2c7..abc84636af07 100644
--- a/arch/arm64/kernel/alternative.c
+++ b/arch/arm64/kernel/alternative.c
@@ -133,8 +133,8 @@ static void clean_dcache_range_nopatch(u64 start, u64 end)
} while (cur += d_size, cur < end);
}
-static void __apply_alternatives(void *alt_region, bool is_module,
-unsigned long *feature_mask)
+static void __nocfi __apply_alternatives(void *alt_region, bool is_module,
+unsigned long *feature_mask)
{
struct alt_instr *alt;
struct alt_region *region = alt_region;
--
2.31.1.295.g9ea45b61b8-goog
[PATCH v6 16/18] arm64: ftrace: use function_nocfi for ftrace_call
With CONFIG_CFI_CLANG, the compiler replaces function pointers with jump table addresses, which breaks dynamic ftrace as the address of ftrace_call is replaced with the address of ftrace_call.cfi_jt. Use function_nocfi() to get the address of the actual function instead. Suggested-by: Ben Dai Signed-off-by: Sami Tolvanen Acked-by: Mark Rutland Tested-by: Nathan Chancellor --- arch/arm64/kernel/ftrace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c index 86a5cf9bc19a..b5d3ddaf69d9 100644 --- a/arch/arm64/kernel/ftrace.c +++ b/arch/arm64/kernel/ftrace.c @@ -55,7 +55,7 @@ int ftrace_update_ftrace_func(ftrace_func_t func) unsigned long pc; u32 new; - pc = (unsigned long)_call; + pc = (unsigned long)function_nocfi(ftrace_call); new = aarch64_insn_gen_branch_imm(pc, (unsigned long)func, AARCH64_INSN_BRANCH_LINK); -- 2.31.1.295.g9ea45b61b8-goog
[PATCH v6 14/18] arm64: add __nocfi to functions that jump to a physical address
Disable CFI checking for functions that switch to linear mapping and
make an indirect call to a physical address, since the compiler only
understands virtual addresses and the CFI check for such indirect calls
would always fail.
Signed-off-by: Sami Tolvanen
Reviewed-by: Kees Cook
Tested-by: Nathan Chancellor
---
arch/arm64/include/asm/mmu_context.h | 2 +-
arch/arm64/kernel/cpu-reset.h| 8
arch/arm64/kernel/cpufeature.c | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/include/asm/mmu_context.h
b/arch/arm64/include/asm/mmu_context.h
index 386b96400a57..d3cef9133539 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -119,7 +119,7 @@ static inline void cpu_install_idmap(void)
* Atomically replaces the active TTBR1_EL1 PGD with a new VA-compatible PGD,
* avoiding the possibility of conflicting TLB entries being allocated.
*/
-static inline void cpu_replace_ttbr1(pgd_t *pgdp)
+static inline void __nocfi cpu_replace_ttbr1(pgd_t *pgdp)
{
typedef void (ttbr_replace_func)(phys_addr_t);
extern ttbr_replace_func idmap_cpu_replace_ttbr1;
diff --git a/arch/arm64/kernel/cpu-reset.h b/arch/arm64/kernel/cpu-reset.h
index f3adc574f969..9a7b1262ef17 100644
--- a/arch/arm64/kernel/cpu-reset.h
+++ b/arch/arm64/kernel/cpu-reset.h
@@ -13,10 +13,10 @@
void __cpu_soft_restart(unsigned long el2_switch, unsigned long entry,
unsigned long arg0, unsigned long arg1, unsigned long arg2);
-static inline void __noreturn cpu_soft_restart(unsigned long entry,
- unsigned long arg0,
- unsigned long arg1,
- unsigned long arg2)
+static inline void __noreturn __nocfi cpu_soft_restart(unsigned long entry,
+ unsigned long arg0,
+ unsigned long arg1,
+ unsigned long arg2)
{
typeof(__cpu_soft_restart) *restart;
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 0b2e0d7b13ec..c2f94a5206e0 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1445,7 +1445,7 @@ static bool unmap_kernel_at_el0(const struct
arm64_cpu_capabilities *entry,
}
#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
-static void
+static void __nocfi
kpti_install_ng_mappings(const struct arm64_cpu_capabilities *__unused)
{
typedef void (kpti_remap_fn)(int, int, phys_addr_t);
--
2.31.1.295.g9ea45b61b8-goog
[PATCH v6 13/18] arm64: use function_nocfi with __pa_symbol
With CONFIG_CFI_CLANG, the compiler replaces function address
references with the address of the function's CFI jump table
entry. This means that __pa_symbol(function) returns the physical
address of the jump table entry, which can lead to address space
confusion as the jump table points to the function's virtual
address. Therefore, use the function_nocfi() macro to ensure we are
always taking the address of the actual function instead.
Signed-off-by: Sami Tolvanen
Reviewed-by: Kees Cook
Acked-by: Mark Rutland
Tested-by: Nathan Chancellor
---
arch/arm64/include/asm/mmu_context.h | 2 +-
arch/arm64/kernel/acpi_parking_protocol.c | 3 ++-
arch/arm64/kernel/cpu-reset.h | 2 +-
arch/arm64/kernel/cpufeature.c| 2 +-
arch/arm64/kernel/psci.c | 3 ++-
arch/arm64/kernel/smp_spin_table.c| 3 ++-
6 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/include/asm/mmu_context.h
b/arch/arm64/include/asm/mmu_context.h
index bd02e99b1a4c..386b96400a57 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -140,7 +140,7 @@ static inline void cpu_replace_ttbr1(pgd_t *pgdp)
ttbr1 |= TTBR_CNP_BIT;
}
- replace_phys = (void *)__pa_symbol(idmap_cpu_replace_ttbr1);
+ replace_phys = (void
*)__pa_symbol(function_nocfi(idmap_cpu_replace_ttbr1));
cpu_install_idmap();
replace_phys(ttbr1);
diff --git a/arch/arm64/kernel/acpi_parking_protocol.c
b/arch/arm64/kernel/acpi_parking_protocol.c
index e7c941d8340d..bfeeb5319abf 100644
--- a/arch/arm64/kernel/acpi_parking_protocol.c
+++ b/arch/arm64/kernel/acpi_parking_protocol.c
@@ -99,7 +99,8 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
* that read this address need to convert this address to the
* Boot-Loader's endianness before jumping.
*/
- writeq_relaxed(__pa_symbol(secondary_entry), >entry_point);
+ writeq_relaxed(__pa_symbol(function_nocfi(secondary_entry)),
+ >entry_point);
writel_relaxed(cpu_entry->gic_cpu_id, >cpu_id);
arch_send_wakeup_ipi_mask(cpumask_of(cpu));
diff --git a/arch/arm64/kernel/cpu-reset.h b/arch/arm64/kernel/cpu-reset.h
index ed50e9587ad8..f3adc574f969 100644
--- a/arch/arm64/kernel/cpu-reset.h
+++ b/arch/arm64/kernel/cpu-reset.h
@@ -22,7 +22,7 @@ static inline void __noreturn cpu_soft_restart(unsigned long
entry,
unsigned long el2_switch = !is_kernel_in_hyp_mode() &&
is_hyp_mode_available();
- restart = (void *)__pa_symbol(__cpu_soft_restart);
+ restart = (void *)__pa_symbol(function_nocfi(__cpu_soft_restart));
cpu_install_idmap();
restart(el2_switch, entry, arg0, arg1, arg2);
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index e5281e1c8f1d..0b2e0d7b13ec 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1462,7 +1462,7 @@ kpti_install_ng_mappings(const struct
arm64_cpu_capabilities *__unused)
if (arm64_use_ng_mappings)
return;
- remap_fn = (void *)__pa_symbol(idmap_kpti_install_ng_mappings);
+ remap_fn = (void
*)__pa_symbol(function_nocfi(idmap_kpti_install_ng_mappings));
cpu_install_idmap();
remap_fn(cpu, num_online_cpus(), __pa_symbol(swapper_pg_dir));
diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
index 62d2bda7adb8..ab7f4c476104 100644
--- a/arch/arm64/kernel/psci.c
+++ b/arch/arm64/kernel/psci.c
@@ -38,7 +38,8 @@ static int __init cpu_psci_cpu_prepare(unsigned int cpu)
static int cpu_psci_cpu_boot(unsigned int cpu)
{
- int err = psci_ops.cpu_on(cpu_logical_map(cpu),
__pa_symbol(secondary_entry));
+ phys_addr_t pa_secondary_entry =
__pa_symbol(function_nocfi(secondary_entry));
+ int err = psci_ops.cpu_on(cpu_logical_map(cpu), pa_secondary_entry);
if (err)
pr_err("failed to boot CPU%d (%d)\n", cpu, err);
diff --git a/arch/arm64/kernel/smp_spin_table.c
b/arch/arm64/kernel/smp_spin_table.c
index 056772c26098..c45a83512805 100644
--- a/arch/arm64/kernel/smp_spin_table.c
+++ b/arch/arm64/kernel/smp_spin_table.c
@@ -66,6 +66,7 @@ static int smp_spin_table_cpu_init(unsigned int cpu)
static int smp_spin_table_cpu_prepare(unsigned int cpu)
{
__le64 __iomem *release_addr;
+ phys_addr_t pa_holding_pen =
__pa_symbol(function_nocfi(secondary_holding_pen));
if (!cpu_release_addr[cpu])
return -ENODEV;
@@ -88,7 +89,7 @@ static int smp_spin_table_cpu_prepare(unsigned int cpu)
* boot-loader's endianness before jumping. This is mandated by
* the boot protocol.
*/
- writeq_relaxed(__pa_symbol(secondary_holding_pen), release_addr);
+ writeq_relaxed(pa_holding_pen, release_addr);
__flush_dcache_area((__force void *)release_addr,
[PATCH v6 12/18] arm64: implement function_nocfi
With CONFIG_CFI_CLANG, the compiler replaces function addresses in
instrumented C code with jump table addresses. This change implements
the function_nocfi() macro, which returns the actual function address
instead.
Signed-off-by: Sami Tolvanen
Reviewed-by: Kees Cook
Acked-by: Mark Rutland
Tested-by: Nathan Chancellor
---
arch/arm64/include/asm/memory.h | 16
1 file changed, 16 insertions(+)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 0aabc3be9a75..3a6f9df63606 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -321,6 +321,22 @@ static inline void *phys_to_virt(phys_addr_t x)
#define virt_to_pfn(x) __phys_to_pfn(__virt_to_phys((unsigned
long)(x)))
#define sym_to_pfn(x) __phys_to_pfn(__pa_symbol(x))
+#ifdef CONFIG_CFI_CLANG
+/*
+ * With CONFIG_CFI_CLANG, the compiler replaces function address
+ * references with the address of the function's CFI jump table
+ * entry. The function_nocfi macro always returns the address of the
+ * actual function instead.
+ */
+#define function_nocfi(x) ({ \
+ void *addr; \
+ asm("adrp %0, " __stringify(x) "\n\t" \
+ "add %0, %0, :lo12:" __stringify(x)\
+ : "=r" (addr)); \
+ addr; \
+})
+#endif
+
/*
* virt_to_page(x)convert a _valid_ virtual address to struct page *
* virt_addr_valid(x) indicates whether a virtual address is valid
--
2.31.1.295.g9ea45b61b8-goog
[PATCH v6 11/18] psci: use function_nocfi for cpu_resume
With CONFIG_CFI_CLANG, the compiler replaces function pointers with
jump table addresses, which results in __pa_symbol returning the
physical address of the jump table entry. As the jump table contains
an immediate jump to an EL1 virtual address, this typically won't
work as intended. Use function_nocfi to get the actual address of
cpu_resume.
Signed-off-by: Sami Tolvanen
Reviewed-by: Kees Cook
Acked-by: Mark Rutland
Tested-by: Nathan Chancellor
---
drivers/firmware/psci/psci.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index f5fc429cae3f..64344e84bd63 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -325,8 +325,9 @@ static int __init psci_features(u32 psci_func_id)
static int psci_suspend_finisher(unsigned long state)
{
u32 power_state = state;
+ phys_addr_t pa_cpu_resume = __pa_symbol(function_nocfi(cpu_resume));
- return psci_ops.cpu_suspend(power_state, __pa_symbol(cpu_resume));
+ return psci_ops.cpu_suspend(power_state, pa_cpu_resume);
}
int psci_cpu_suspend_enter(u32 state)
@@ -344,8 +345,10 @@ int psci_cpu_suspend_enter(u32 state)
static int psci_system_suspend(unsigned long unused)
{
+ phys_addr_t pa_cpu_resume = __pa_symbol(function_nocfi(cpu_resume));
+
return invoke_psci_fn(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND),
- __pa_symbol(cpu_resume), 0, 0);
+ pa_cpu_resume, 0, 0);
}
static int psci_system_suspend_enter(suspend_state_t state)
--
2.31.1.295.g9ea45b61b8-goog
[PATCH v6 10/18] lkdtm: use function_nocfi
To ensure we take the actual address of a function in kernel text,
use function_nocfi. Otherwise, with CONFIG_CFI_CLANG, the compiler
replaces the address with a pointer to the CFI jump table, which is
actually in the module when compiled with CONFIG_LKDTM=m.
Signed-off-by: Sami Tolvanen
Acked-by: Kees Cook
Tested-by: Nathan Chancellor
---
drivers/misc/lkdtm/usercopy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/misc/lkdtm/usercopy.c b/drivers/misc/lkdtm/usercopy.c
index 109e8d4302c1..15d220ef35a5 100644
--- a/drivers/misc/lkdtm/usercopy.c
+++ b/drivers/misc/lkdtm/usercopy.c
@@ -314,7 +314,7 @@ void lkdtm_USERCOPY_KERNEL(void)
pr_info("attempting bad copy_to_user from kernel text: %px\n",
vm_mmap);
- if (copy_to_user((void __user *)user_addr, vm_mmap,
+ if (copy_to_user((void __user *)user_addr, function_nocfi(vm_mmap),
unconst + PAGE_SIZE)) {
pr_warn("copy_to_user failed, but lacked Oops\n");
goto free_user;
--
2.31.1.295.g9ea45b61b8-goog
[PATCH v6 09/18] treewide: Change list_sort to use const pointers
list_sort() internally casts the comparison function passed to it
to a different type with constant struct list_head pointers, and
uses this pointer to call the functions, which trips indirect call
Control-Flow Integrity (CFI) checking.
Instead of removing the consts, this change defines the
list_cmp_func_t type and changes the comparison function types of
all list_sort() callers to use const pointers, thus avoiding type
mismatches.
Suggested-by: Nick Desaulniers
Signed-off-by: Sami Tolvanen
Reviewed-by: Nick Desaulniers
Reviewed-by: Christoph Hellwig
Reviewed-by: Kees Cook
Tested-by: Nick Desaulniers
Tested-by: Nathan Chancellor
---
arch/arm64/kvm/vgic/vgic-its.c | 8
arch/arm64/kvm/vgic/vgic.c | 3 ++-
block/blk-mq-sched.c| 3 ++-
block/blk-mq.c | 3 ++-
drivers/acpi/nfit/core.c| 3 ++-
drivers/acpi/numa/hmat.c| 3 ++-
drivers/clk/keystone/sci-clk.c | 4 ++--
drivers/gpu/drm/drm_modes.c | 3 ++-
drivers/gpu/drm/i915/gt/intel_engine_user.c | 3 ++-
drivers/gpu/drm/i915/gvt/debugfs.c | 2 +-
drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 3 ++-
drivers/gpu/drm/radeon/radeon_cs.c | 4 ++--
.../hw/usnic/usnic_uiom_interval_tree.c | 3 ++-
drivers/interconnect/qcom/bcm-voter.c | 2 +-
drivers/md/raid5.c | 3 ++-
drivers/misc/sram.c | 4 ++--
drivers/nvme/host/core.c| 3 ++-
.../pci/controller/cadence/pcie-cadence-host.c | 3 ++-
drivers/spi/spi-loopback-test.c | 3 ++-
fs/btrfs/raid56.c | 3 ++-
fs/btrfs/tree-log.c | 3 ++-
fs/btrfs/volumes.c | 3 ++-
fs/ext4/fsmap.c | 4 ++--
fs/gfs2/glock.c | 3 ++-
fs/gfs2/log.c | 2 +-
fs/gfs2/lops.c | 3 ++-
fs/iomap/buffered-io.c | 3 ++-
fs/ubifs/gc.c | 7 ---
fs/ubifs/replay.c | 4 ++--
fs/xfs/scrub/bitmap.c | 4 ++--
fs/xfs/xfs_bmap_item.c | 4 ++--
fs/xfs/xfs_buf.c| 6 +++---
fs/xfs/xfs_extent_busy.c| 4 ++--
fs/xfs/xfs_extent_busy.h| 3 ++-
fs/xfs/xfs_extfree_item.c | 4 ++--
fs/xfs/xfs_refcount_item.c | 4 ++--
fs/xfs/xfs_rmap_item.c | 4 ++--
include/linux/list_sort.h | 7 ---
lib/list_sort.c | 17 ++---
lib/test_list_sort.c| 3 ++-
net/tipc/name_table.c | 4 ++--
41 files changed, 90 insertions(+), 72 deletions(-)
diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index 40cbaca81333..b9518f94bd43 100644
--- a/arch/arm64/kvm/vgic/vgic-its.c
+++ b/arch/arm64/kvm/vgic/vgic-its.c
@@ -2190,8 +2190,8 @@ static int vgic_its_restore_ite(struct vgic_its *its, u32
event_id,
return offset;
}
-static int vgic_its_ite_cmp(void *priv, struct list_head *a,
- struct list_head *b)
+static int vgic_its_ite_cmp(void *priv, const struct list_head *a,
+ const struct list_head *b)
{
struct its_ite *itea = container_of(a, struct its_ite, ite_list);
struct its_ite *iteb = container_of(b, struct its_ite, ite_list);
@@ -2329,8 +2329,8 @@ static int vgic_its_restore_dte(struct vgic_its *its, u32
id,
return offset;
}
-static int vgic_its_device_cmp(void *priv, struct list_head *a,
- struct list_head *b)
+static int vgic_its_device_cmp(void *priv, const struct list_head *a,
+ const struct list_head *b)
{
struct its_device *deva = container_of(a, struct its_device, dev_list);
struct its_device *devb = container_of(b, struct its_device, dev_list);
diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
index 1c597c9885fa..15b666200f0b 100644
--- a/arch/arm64/kvm/vgic/vgic.c
+++ b/arch/arm64/kvm/vgic/vgic.c
@@ -255,7 +255,8 @@ static struct kvm_vcpu *vgic_target_oracle(struct vgic_irq
*irq)
* Return negative if "a" sorts before "b", 0 to preserve order, and positive
* to sort "b" before "a".
*/
-static int vgic_irq_cmp(void *priv, struct list_head *a, struct list_head *b)
+static int vgic_irq_cmp(void *priv, const struct list_head *a,
+ const struct list_head *b)
{
struct vgic_irq *irqa = container_of(a, s
[PATCH v6 08/18] bpf: disable CFI in dispatcher functions
BPF dispatcher functions are patched at runtime to perform direct
instead of indirect calls. Disable CFI for the dispatcher functions to
avoid conflicts.
Signed-off-by: Sami Tolvanen
Reviewed-by: Kees Cook
Tested-by: Nathan Chancellor
---
include/linux/bpf.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 3625f019767d..2f46f98479e1 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -650,7 +650,7 @@ struct bpf_dispatcher {
struct bpf_ksym ksym;
};
-static __always_inline unsigned int bpf_dispatcher_nop_func(
+static __always_inline __nocfi unsigned int bpf_dispatcher_nop_func(
const void *ctx,
const struct bpf_insn *insnsi,
unsigned int (*bpf_func)(const void *,
@@ -678,7 +678,7 @@ void bpf_trampoline_put(struct bpf_trampoline *tr);
}
#define DEFINE_BPF_DISPATCHER(name)\
- noinline unsigned int bpf_dispatcher_##name##_func( \
+ noinline __nocfi unsigned int bpf_dispatcher_##name##_func( \
const void *ctx,\
const struct bpf_insn *insnsi, \
unsigned int (*bpf_func)(const void *, \
--
2.31.1.295.g9ea45b61b8-goog
[PATCH v6 07/18] kallsyms: strip ThinLTO hashes from static functions
With CONFIG_CFI_CLANG and ThinLTO, Clang appends a hash to the names
of all static functions not marked __used. This can break userspace
tools that don't expect the function name to change, so strip out the
hash from the output.
Suggested-by: Jack Pham
Signed-off-by: Sami Tolvanen
Reviewed-by: Kees Cook
Tested-by: Nathan Chancellor
---
kernel/kallsyms.c | 55 ++-
1 file changed, 50 insertions(+), 5 deletions(-)
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 8043a90aa50e..c851ca0ed357 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -161,6 +161,27 @@ static unsigned long kallsyms_sym_address(int idx)
return kallsyms_relative_base - 1 - kallsyms_offsets[idx];
}
+#if defined(CONFIG_CFI_CLANG) && defined(CONFIG_LTO_CLANG_THIN)
+/*
+ * LLVM appends a hash to static function names when ThinLTO and CFI are
+ * both enabled, i.e. foo() becomes foo$707af9a22804d33c81801f27dcfe489b.
+ * This causes confusion and potentially breaks user space tools, so we
+ * strip the suffix from expanded symbol names.
+ */
+static inline bool cleanup_symbol_name(char *s)
+{
+ char *res;
+
+ res = strrchr(s, '$');
+ if (res)
+ *res = '\0';
+
+ return res != NULL;
+}
+#else
+static inline bool cleanup_symbol_name(char *s) { return false; }
+#endif
+
/* Lookup the address for this symbol. Returns 0 if not found. */
unsigned long kallsyms_lookup_name(const char *name)
{
@@ -173,6 +194,9 @@ unsigned long kallsyms_lookup_name(const char *name)
if (strcmp(namebuf, name) == 0)
return kallsyms_sym_address(i);
+
+ if (cleanup_symbol_name(namebuf) && strcmp(namebuf, name) == 0)
+ return kallsyms_sym_address(i);
}
return module_kallsyms_lookup_name(name);
}
@@ -303,7 +327,9 @@ const char *kallsyms_lookup(unsigned long addr,
namebuf, KSYM_NAME_LEN);
if (modname)
*modname = NULL;
- return namebuf;
+
+ ret = namebuf;
+ goto found;
}
/* See if it's in a module or a BPF JITed image. */
@@ -316,11 +342,16 @@ const char *kallsyms_lookup(unsigned long addr,
if (!ret)
ret = ftrace_mod_address_lookup(addr, symbolsize,
offset, modname, namebuf);
+
+found:
+ cleanup_symbol_name(namebuf);
return ret;
}
int lookup_symbol_name(unsigned long addr, char *symname)
{
+ int res;
+
symname[0] = '\0';
symname[KSYM_NAME_LEN - 1] = '\0';
@@ -331,15 +362,23 @@ int lookup_symbol_name(unsigned long addr, char *symname)
/* Grab name */
kallsyms_expand_symbol(get_symbol_offset(pos),
symname, KSYM_NAME_LEN);
- return 0;
+ goto found;
}
/* See if it's in a module. */
- return lookup_module_symbol_name(addr, symname);
+ res = lookup_module_symbol_name(addr, symname);
+ if (res)
+ return res;
+
+found:
+ cleanup_symbol_name(symname);
+ return 0;
}
int lookup_symbol_attrs(unsigned long addr, unsigned long *size,
unsigned long *offset, char *modname, char *name)
{
+ int res;
+
name[0] = '\0';
name[KSYM_NAME_LEN - 1] = '\0';
@@ -351,10 +390,16 @@ int lookup_symbol_attrs(unsigned long addr, unsigned long
*size,
kallsyms_expand_symbol(get_symbol_offset(pos),
name, KSYM_NAME_LEN);
modname[0] = '\0';
- return 0;
+ goto found;
}
/* See if it's in a module. */
- return lookup_module_symbol_attrs(addr, size, offset, modname, name);
+ res = lookup_module_symbol_attrs(addr, size, offset, modname, name);
+ if (res)
+ return res;
+
+found:
+ cleanup_symbol_name(name);
+ return 0;
}
/* Look up a kernel symbol and return it in a text buffer. */
--
2.31.1.295.g9ea45b61b8-goog
[PATCH v6 06/18] kthread: use WARN_ON_FUNCTION_MISMATCH
With CONFIG_CFI_CLANG, a callback function passed to __kthread_queue_delayed_work from a module points to a jump table entry defined in the module instead of the one used in the core kernel, which breaks function address equality in this check: WARN_ON_ONCE(timer->function != ktead_delayed_work_timer_fn); Use WARN_ON_FUNCTION_MISMATCH() instead to disable the warning when CFI and modules are both enabled. Signed-off-by: Sami Tolvanen Reviewed-by: Kees Cook Tested-by: Nathan Chancellor --- kernel/kthread.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kernel/kthread.c b/kernel/kthread.c index 1578973c5740..a1972eba2917 100644 --- a/kernel/kthread.c +++ b/kernel/kthread.c @@ -963,7 +963,8 @@ static void __kthread_queue_delayed_work(struct kthread_worker *worker, struct timer_list *timer = >timer; struct kthread_work *work = >work; - WARN_ON_ONCE(timer->function != kthread_delayed_work_timer_fn); + WARN_ON_FUNCTION_MISMATCH(timer->function, + kthread_delayed_work_timer_fn); /* * If @delay is 0, queue @dwork->work immediately. This is for -- 2.31.1.295.g9ea45b61b8-goog
[PATCH v6 05/18] workqueue: use WARN_ON_FUNCTION_MISMATCH
With CONFIG_CFI_CLANG, a callback function passed to __queue_delayed_work from a module points to a jump table entry defined in the module instead of the one used in the core kernel, which breaks function address equality in this check: WARN_ON_ONCE(timer->function != delayed_work_timer_fn); Use WARN_ON_FUNCTION_MISMATCH() instead to disable the warning when CFI and modules are both enabled. Signed-off-by: Sami Tolvanen Reviewed-by: Kees Cook Tested-by: Nathan Chancellor --- kernel/workqueue.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 79f2319543ce..b19d759e55a5 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -1630,7 +1630,7 @@ static void __queue_delayed_work(int cpu, struct workqueue_struct *wq, struct work_struct *work = >work; WARN_ON_ONCE(!wq); - WARN_ON_ONCE(timer->function != delayed_work_timer_fn); + WARN_ON_FUNCTION_MISMATCH(timer->function, delayed_work_timer_fn); WARN_ON_ONCE(timer_pending(timer)); WARN_ON_ONCE(!list_empty(>entry)); -- 2.31.1.295.g9ea45b61b8-goog
[PATCH v6 04/18] module: ensure __cfi_check alignment
CONFIG_CFI_CLANG_SHADOW assumes the __cfi_check() function is page
aligned and at the beginning of the .text section. While Clang would
normally align the function correctly, it fails to do so for modules
with no executable code.
This change ensures the correct __cfi_check() location and
alignment. It also discards the .eh_frame section, which Clang can
generate with certain sanitizers, such as CFI.
Link: https://bugs.llvm.org/show_bug.cgi?id=46293
Signed-off-by: Sami Tolvanen
Reviewed-by: Kees Cook
Acked-by: Jessica Yu
Tested-by: Nathan Chancellor
---
scripts/module.lds.S | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/scripts/module.lds.S b/scripts/module.lds.S
index 2c52535f9b56..04c5685c25cf 100644
--- a/scripts/module.lds.S
+++ b/scripts/module.lds.S
@@ -3,10 +3,20 @@
* Archs are free to supply their own linker scripts. ld will
* combine them automatically.
*/
+#ifdef CONFIG_CFI_CLANG
+# include
+# define ALIGN_CFI ALIGN(PAGE_SIZE)
+# define SANITIZER_DISCARDS*(.eh_frame)
+#else
+# define ALIGN_CFI
+# define SANITIZER_DISCARDS
+#endif
+
SECTIONS {
/DISCARD/ : {
*(.discard)
*(.discard.*)
+ SANITIZER_DISCARDS
}
__ksymtab 0 : { *(SORT(___ksymtab+*)) }
@@ -41,7 +51,14 @@ SECTIONS {
*(.rodata..L*)
}
- .text : { *(.text .text.[0-9a-zA-Z_]*) }
+ /*
+* With CONFIG_CFI_CLANG, we assume __cfi_check is at the beginning
+* of the .text section, and is aligned to PAGE_SIZE.
+*/
+ .text : ALIGN_CFI {
+ *(.text.__cfi_check)
+ *(.text .text.[0-9a-zA-Z_]* .text..L.cfi*)
+ }
#endif
}
--
2.31.1.295.g9ea45b61b8-goog
[PATCH v6 03/18] mm: add generic function_nocfi macro
With CONFIG_CFI_CLANG, the compiler replaces function addresses in instrumented C code with jump table addresses. This means that __pa_symbol(function) returns the physical address of the jump table entry instead of the actual function, which may not work as the jump table code will immediately jump to a virtual address that may not be mapped. To avoid this address space confusion, this change adds a generic definition for function_nocfi(), which architectures that support CFI can override. The typical implementation of would use inline assembly to take the function address, which avoids compiler instrumentation. Signed-off-by: Sami Tolvanen Reviewed-by: Kees Cook Acked-by: Mark Rutland Tested-by: Nathan Chancellor --- include/linux/mm.h | 10 ++ 1 file changed, 10 insertions(+) diff --git a/include/linux/mm.h b/include/linux/mm.h index 8ba434287387..22cce9c7dd05 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -124,6 +124,16 @@ extern int mmap_rnd_compat_bits __read_mostly; #define lm_alias(x)__va(__pa_symbol(x)) #endif +/* + * With CONFIG_CFI_CLANG, the compiler replaces function addresses in + * instrumented C code with jump table addresses. Architectures that + * support CFI can define this macro to return the actual function address + * when needed. + */ +#ifndef function_nocfi +#define function_nocfi(x) (x) +#endif + /* * To prevent common memory management code establishing * a zero page mapping on a read fault. -- 2.31.1.295.g9ea45b61b8-goog
[PATCH v6 02/18] cfi: add __cficanonical
With CONFIG_CFI_CLANG, the compiler replaces a function address taken
in C code with the address of a local jump table entry, which passes
runtime indirect call checks. However, the compiler won't replace
addresses taken in assembly code, which will result in a CFI failure
if we later jump to such an address in instrumented C code. The code
generated for the non-canonical jump table looks this:
: /* In C, points here */
jmp noncanonical
...
:/* function body */
...
This change adds the __cficanonical attribute, which tells the
compiler to use a canonical jump table for the function instead. This
means the compiler will rename the actual function to .cfi
and points the original symbol to the jump table entry instead:
: /* jump table entry */
jmp canonical.cfi
...
: /* function body */
...
As a result, the address taken in assembly, or other non-instrumented
code always points to the jump table and therefore, can be used for
indirect calls in instrumented code without tripping CFI checks.
Signed-off-by: Sami Tolvanen
Reviewed-by: Kees Cook
Acked-by: Bjorn Helgaas# pci.h
Tested-by: Nathan Chancellor
---
include/linux/compiler-clang.h | 1 +
include/linux/compiler_types.h | 4
include/linux/init.h | 4 ++--
include/linux/pci.h| 4 ++--
4 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index 6de9d0c9377e..adbe76b203e2 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -63,3 +63,4 @@
#endif
#define __nocfi__attribute__((__no_sanitize__("cfi")))
+#define __cficanonical __attribute__((__cfi_canonical_jump_table__))
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 796935a37e37..d29bda7f6ebd 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -246,6 +246,10 @@ struct ftrace_likely_data {
# define __nocfi
#endif
+#ifndef __cficanonical
+# define __cficanonical
+#endif
+
#ifndef asm_volatile_goto
#define asm_volatile_goto(x...) asm goto(x)
#endif
diff --git a/include/linux/init.h b/include/linux/init.h
index b3ea15348fbd..045ad1650ed1 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -220,8 +220,8 @@ extern bool initcall_debug;
__initcall_name(initstub, __iid, id)
#define __define_initcall_stub(__stub, fn) \
- int __init __stub(void);\
- int __init __stub(void) \
+ int __init __cficanonical __stub(void); \
+ int __init __cficanonical __stub(void) \
{ \
return fn();\
} \
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 86c799c97b77..39684b72db91 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1944,8 +1944,8 @@ enum pci_fixup_pass {
#ifdef CONFIG_LTO_CLANG
#define __DECLARE_PCI_FIXUP_SECTION(sec, name, vendor, device, class, \
class_shift, hook, stub) \
- void stub(struct pci_dev *dev); \
- void stub(struct pci_dev *dev) \
+ void __cficanonical stub(struct pci_dev *dev); \
+ void __cficanonical stub(struct pci_dev *dev) \
{ \
hook(dev); \
} \
--
2.31.1.295.g9ea45b61b8-goog
[PATCH v6 01/18] add support for Clang CFI
This change adds support for Clang’s forward-edge Control Flow Integrity (CFI) checking. With CONFIG_CFI_CLANG, the compiler injects a runtime check before each indirect function call to ensure the target is a valid function with the correct static type. This restricts possible call targets and makes it more difficult for an attacker to exploit bugs that allow the modification of stored function pointers. For more details, see: https://clang.llvm.org/docs/ControlFlowIntegrity.html Clang requires CONFIG_LTO_CLANG to be enabled with CFI to gain visibility to possible call targets. Kernel modules are supported with Clang’s cross-DSO CFI mode, which allows checking between independently compiled components. With CFI enabled, the compiler injects a __cfi_check() function into the kernel and each module for validating local call targets. For cross-module calls that cannot be validated locally, the compiler calls the global __cfi_slowpath_diag() function, which determines the target module and calls the correct __cfi_check() function. This patch includes a slowpath implementation that uses __module_address() to resolve call targets, and with CONFIG_CFI_CLANG_SHADOW enabled, a shadow map that speeds up module look-ups by ~3x. Clang implements indirect call checking using jump tables and offers two methods of generating them. With canonical jump tables, the compiler renames each address-taken function to .cfi and points the original symbol to a jump table entry, which passes __cfi_check() validation. This isn’t compatible with stand-alone assembly code, which the compiler doesn’t instrument, and would result in indirect calls to assembly code to fail. Therefore, we default to using non-canonical jump tables instead, where the compiler generates a local jump table entry .cfi_jt for each address-taken function, and replaces all references to the function with the address of the jump table entry. Note that because non-canonical jump table addresses are local to each component, they break cross-module function address equality. Specifically, the address of a global function will be different in each module, as it's replaced with the address of a local jump table entry. If this address is passed to a different module, it won’t match the address of the same function taken there. This may break code that relies on comparing addresses passed from other components. CFI checking can be disabled in a function with the __nocfi attribute. Additionally, CFI can be disabled for an entire compilation unit by filtering out CC_FLAGS_CFI. By default, CFI failures result in a kernel panic to stop a potential exploit. CONFIG_CFI_PERMISSIVE enables a permissive mode, where the kernel prints out a rate-limited warning instead, and allows execution to continue. This option is helpful for locating type mismatches, but should only be enabled during development. Signed-off-by: Sami Tolvanen Reviewed-by: Kees Cook Tested-by: Nathan Chancellor --- Makefile | 17 ++ arch/Kconfig | 45 include/asm-generic/bug.h | 16 ++ include/asm-generic/vmlinux.lds.h | 20 +- include/linux/cfi.h | 41 include/linux/compiler-clang.h| 2 + include/linux/compiler_types.h| 4 + include/linux/init.h | 2 +- include/linux/module.h| 13 +- init/Kconfig | 2 +- kernel/Makefile | 4 + kernel/cfi.c | 329 ++ kernel/module.c | 43 scripts/Makefile.modfinal | 2 +- 14 files changed, 534 insertions(+), 6 deletions(-) create mode 100644 include/linux/cfi.h create mode 100644 kernel/cfi.c diff --git a/Makefile b/Makefile index cc77fd45ca64..427f75249d5e 100644 --- a/Makefile +++ b/Makefile @@ -920,6 +920,23 @@ KBUILD_AFLAGS += -fno-lto export CC_FLAGS_LTO endif +ifdef CONFIG_CFI_CLANG +CC_FLAGS_CFI := -fsanitize=cfi \ + -fsanitize-cfi-cross-dso \ + -fno-sanitize-cfi-canonical-jump-tables \ + -fno-sanitize-trap=cfi \ + -fno-sanitize-blacklist + +ifdef CONFIG_CFI_PERMISSIVE +CC_FLAGS_CFI += -fsanitize-recover=cfi +endif + +# If LTO flags are filtered out, we must also filter out CFI. +CC_FLAGS_LTO += $(CC_FLAGS_CFI) +KBUILD_CFLAGS += $(CC_FLAGS_CFI) +export CC_FLAGS_CFI +endif + ifdef CONFIG_DEBUG_FORCE_FUNCTION_ALIGN_32B KBUILD_CFLAGS += -falign-functions=32 endif diff --git a/arch/Kconfig b/arch/Kconfig index ecfd3520b676..f6a85ba6cba2 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -692,6 +692,51 @@ config LTO_CLANG_THIN If unsure, say Y. endchoice +config ARCH_SUPPORTS_CFI_CLANG + bool + help + An architecture should select this option if it can support Clang's + Control-Flow Integrity (CFI) checking. + +config CFI_CLANG + bool "Use Clang's Control Flow Integrity
[PATCH v6 00/18] Add support for Clang CFI
This series adds support for Clang's Control-Flow Integrity (CFI) checking. With CFI, the compiler injects a runtime check before each indirect function call to ensure the target is a valid function with the correct static type. This restricts possible call targets and makes it more difficult for an attacker to exploit bugs that allow the modification of stored function pointers. For more details, see: https://clang.llvm.org/docs/ControlFlowIntegrity.html The first patch contains build system changes and error handling, and implements support for cross-module indirect call checking. The remaining patches address issues caused by the compiler instrumentation. These include fixing known type mismatches, as well as issues with address space confusion and cross-module function address equality. These patches add support only for arm64, but I'll post patches also for x86_64 after we address the remaining issues there, including objtool support. You can also pull this series from https://github.com/samitolvanen/linux.git cfi-v6 --- Changes in v6: - Added temporary variables and moved assembly constraints to a separate line based on Mark's suggestions. Changes in v5: - Changed module.lds.S to only include when CFI is enabled to fix the MIPS build. - Added a patch that fixes dynamic ftrace with CFI on arm64. Changes in v4: - Per Mark's suggestion, dropped __pa_function() and renamed __va_function() to function_nocfi(). - Added a comment to function_nocfi() to explain what it does. - Updated the psci patch to use an intermediate variable for the physical address for clarity. Changes in v3: - Added a patch to change list_sort() callers treewide to use const pointers instead of simply removing the internal casts. - Changed cleanup_symbol_name() to return bool. - Changed module.lds.S to drop the .eh_frame section only with CONFIG_CFI_CLANG. - Switched to synchronize_rcu() in update_shadow(). Changes in v2: - Fixed .text merging in module.lds.S. - Added WARN_ON_FUNCTION_MISMATCH() and changed kernel/thread.c and kernel/workqueue.c to use the macro instead. Sami Tolvanen (18): add support for Clang CFI cfi: add __cficanonical mm: add generic function_nocfi macro module: ensure __cfi_check alignment workqueue: use WARN_ON_FUNCTION_MISMATCH kthread: use WARN_ON_FUNCTION_MISMATCH kallsyms: strip ThinLTO hashes from static functions bpf: disable CFI in dispatcher functions treewide: Change list_sort to use const pointers lkdtm: use function_nocfi psci: use function_nocfi for cpu_resume arm64: implement function_nocfi arm64: use function_nocfi with __pa_symbol arm64: add __nocfi to functions that jump to a physical address arm64: add __nocfi to __apply_alternatives arm64: ftrace: use function_nocfi for ftrace_call KVM: arm64: Disable CFI for nVHE arm64: allow CONFIG_CFI_CLANG to be selected Makefile | 17 + arch/Kconfig | 45 +++ arch/arm64/Kconfig| 1 + arch/arm64/include/asm/memory.h | 16 + arch/arm64/include/asm/mmu_context.h | 4 +- arch/arm64/kernel/acpi_parking_protocol.c | 3 +- arch/arm64/kernel/alternative.c | 4 +- arch/arm64/kernel/cpu-reset.h | 10 +- arch/arm64/kernel/cpufeature.c| 4 +- arch/arm64/kernel/ftrace.c| 2 +- arch/arm64/kernel/psci.c | 3 +- arch/arm64/kernel/smp_spin_table.c| 3 +- arch/arm64/kvm/hyp/nvhe/Makefile | 6 +- arch/arm64/kvm/vgic/vgic-its.c| 8 +- arch/arm64/kvm/vgic/vgic.c| 3 +- block/blk-mq-sched.c | 3 +- block/blk-mq.c| 3 +- drivers/acpi/nfit/core.c | 3 +- drivers/acpi/numa/hmat.c | 3 +- drivers/clk/keystone/sci-clk.c| 4 +- drivers/firmware/psci/psci.c | 7 +- drivers/gpu/drm/drm_modes.c | 3 +- drivers/gpu/drm/i915/gt/intel_engine_user.c | 3 +- drivers/gpu/drm/i915/gvt/debugfs.c| 2 +- drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 3 +- drivers/gpu/drm/radeon/radeon_cs.c| 4 +- .../hw/usnic/usnic_uiom_interval_tree.c | 3 +- drivers/interconnect/qcom/bcm-voter.c | 2 +- drivers/md/raid5.c| 3 +- drivers/misc/lkdtm/usercopy.c | 2 +- drivers/misc/sram.c | 4 +- drivers/nvme/host/core.c | 3 +- .../controller/cadence/pcie-cadence-host.c| 3 +- drivers/spi/spi-loopback-test.c | 3 +- fs/btrfs/raid56.c | 3 +- fs/btrfs/tree-log.c | 3 +- fs/btrfs/volumes.c| 3 +- fs/ext4/fsmap.c
Re: [PATCH v5 14/18] arm64: add __nocfi to functions that jump to a physical address
On Tue, Apr 6, 2021 at 4:54 AM Mark Rutland wrote: > > [adding Ard for EFI runtime services bits] > > On Thu, Apr 01, 2021 at 04:32:12PM -0700, Sami Tolvanen wrote: > > Disable CFI checking for functions that switch to linear mapping and > > make an indirect call to a physical address, since the compiler only > > understands virtual addresses and the CFI check for such indirect calls > > would always fail. > > What does physical vs virtual have to do with this? Does the address > actually matter, or is this just a general thing that when calling an > assembly function we won't have a trampoline that the caller expects? No, this is about the actual address. The compiler-generated runtime checks only know about EL1 virtual addresses, so if we switch to a different address space, all indirect calls will trip CFI. > I wonder if we need to do something with asmlinkage here, perhaps? > > I didn't spot anything in the seriues handling EFI runtime services > calls, and I strongly suspect we need to do something for those, unless > they're handled implicitly by something else. > > > Signed-off-by: Sami Tolvanen > > Reviewed-by: Kees Cook > > --- > > arch/arm64/include/asm/mmu_context.h | 2 +- > > arch/arm64/kernel/cpu-reset.h| 8 > > arch/arm64/kernel/cpufeature.c | 2 +- > > 3 files changed, 6 insertions(+), 6 deletions(-) > >https://www.cnbc.com/2021/04/06/donald-trump-save-america-pac-has-85-million-on-hand-ahead-of-midterms.html > > diff --git a/arch/arm64/include/asm/mmu_context.h > > b/arch/arm64/include/asm/mmu_context.h > > index 386b96400a57..d3cef9133539 100644 > > --- a/arch/arm64/include/asm/mmu_context.h > > +++ b/arch/arm64/include/asm/mmu_context.h > > @@ -119,7 +119,7 @@ static inline void cpu_install_idmap(void) > > * Atomically replaces the active TTBR1_EL1 PGD with a new VA-compatible > > PGD, > > * avoiding the possibility of conflicting TLB entries being allocated. > > */ > > -static inline void cpu_replace_ttbr1(pgd_t *pgdp) > > +static inline void __nocfi cpu_replace_ttbr1(pgd_t *pgdp) > > Given these are inlines, what's the effect when these are inlined into a > function that would normally use CFI? Does CFI get supressed for the > whole function, or just the bit that got inlined? Just for the bit that gets inlined. > Is there an attribute that we could place on a function pointer to tell > the compiler to not check calls via that pointer? If that existed we'd > be able to scope this much more tightly. There isn't, but I do agree that this would be a useful feature. Sami
Re: [PATCH v5 12/18] arm64: implement function_nocfi
On Tue, Apr 6, 2021 at 4:37 AM Mark Rutland wrote:
>
> On Thu, Apr 01, 2021 at 04:32:10PM -0700, Sami Tolvanen wrote:
> > With CONFIG_CFI_CLANG, the compiler replaces function addresses in
> > instrumented C code with jump table addresses. This change implements
> > the function_nocfi() macro, which returns the actual function address
> > instead.
> >
> > Signed-off-by: Sami Tolvanen
> > Reviewed-by: Kees Cook
>
> I think that it's unfortunate that we have to drop to assembly here, but
> given this is infrequent I agree it's not the end of the world, so:
>
> Acked-by: Mark Rutland
>
> > ---
> > arch/arm64/include/asm/memory.h | 15 +++
> > 1 file changed, 15 insertions(+)
> >
> > diff --git a/arch/arm64/include/asm/memory.h
> > b/arch/arm64/include/asm/memory.h
> > index 0aabc3be9a75..b55410afd3d1 100644
> > --- a/arch/arm64/include/asm/memory.h
> > +++ b/arch/arm64/include/asm/memory.h
> > @@ -321,6 +321,21 @@ static inline void *phys_to_virt(phys_addr_t x)
> > #define virt_to_pfn(x)
> > __phys_to_pfn(__virt_to_phys((unsigned long)(x)))
> > #define sym_to_pfn(x)__phys_to_pfn(__pa_symbol(x))
> >
> > +#ifdef CONFIG_CFI_CLANG
> > +/*
> > + * With CONFIG_CFI_CLANG, the compiler replaces function address
> > + * references with the address of the function's CFI jump table
> > + * entry. The function_nocfi macro always returns the address of the
> > + * actual function instead.
> > + */
> > +#define function_nocfi(x) ({ \
> > + void *addr; \
> > + asm("adrp %0, " __stringify(x) "\n\t" \
> > + "add %0, %0, :lo12:" __stringify(x) : "=r" (addr));\
>
> If it's not too painful, could we please move the asm constrain onto its
> own line? That makes it slightly easier to read, and aligns with what
> we've (mostly) done elsewhere in arm64.
Sure, I'll change this in the next version.
> Not a big deal either way, and the ack stands regardless.
>
> Thanks,
> Mark.
Sami
Re: [PATCH] kbuild: merge module sections under CONFIG_LD_DEAD_CODE_DATA_ELIMINATION too
On Fri, Apr 2, 2021 at 5:40 AM Alexander Lobakin wrote:
>
> When building with CONFIG_LD_DEAD_CODE_DATA_ELIMINATION,
> -fdata-sections and -ffunction-sections are being enabled by the
> top-level Makefile, and module section merging is also needed.
> Expand the ifdef (and the comment block) to cover that case too.
>
> Fixes: 6a3193cdd5e5 ("kbuild: lto: Merge module sections if and only if
> CONFIG_LTO_CLANG is enabled")
Wouldn't this trigger the ld.bfd bug described in the commit message
when LD_DEAD_CODE_DATA_ELIMINATION is enabled? LTO_CLANG always uses
LLD, so it won't have this issue.
Sami
[PATCH v5 15/18] arm64: add __nocfi to __apply_alternatives
__apply_alternatives makes indirect calls to functions whose address
is taken in assembly code using the alternative_cb macro. With
non-canonical CFI, the compiler won't replace these function
references with the jump table addresses, which trips CFI. Disable CFI
checking in the function to work around the issue.
Signed-off-by: Sami Tolvanen
Reviewed-by: Kees Cook
---
arch/arm64/kernel/alternative.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/alternative.c b/arch/arm64/kernel/alternative.c
index 1184c44ea2c7..abc84636af07 100644
--- a/arch/arm64/kernel/alternative.c
+++ b/arch/arm64/kernel/alternative.c
@@ -133,8 +133,8 @@ static void clean_dcache_range_nopatch(u64 start, u64 end)
} while (cur += d_size, cur < end);
}
-static void __apply_alternatives(void *alt_region, bool is_module,
-unsigned long *feature_mask)
+static void __nocfi __apply_alternatives(void *alt_region, bool is_module,
+unsigned long *feature_mask)
{
struct alt_instr *alt;
struct alt_region *region = alt_region;
--
2.31.0.208.g409f899ff0-goog
[PATCH v5 16/18] arm64: ftrace: use function_nocfi for ftrace_call
With CONFIG_CFI_CLANG, the compiler replaces function pointers with jump table addresses, which breaks dynamic ftrace as the address of ftrace_call is replaced with the address of ftrace_call.cfi_jt. Use function_nocfi() to get the address of the actual function instead. Suggested-by: Ben Dai Signed-off-by: Sami Tolvanen --- arch/arm64/kernel/ftrace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c index 86a5cf9bc19a..b5d3ddaf69d9 100644 --- a/arch/arm64/kernel/ftrace.c +++ b/arch/arm64/kernel/ftrace.c @@ -55,7 +55,7 @@ int ftrace_update_ftrace_func(ftrace_func_t func) unsigned long pc; u32 new; - pc = (unsigned long)_call; + pc = (unsigned long)function_nocfi(ftrace_call); new = aarch64_insn_gen_branch_imm(pc, (unsigned long)func, AARCH64_INSN_BRANCH_LINK); -- 2.31.0.208.g409f899ff0-goog
[PATCH v5 17/18] KVM: arm64: Disable CFI for nVHE
Disable CFI for the nVHE code to avoid address space confusion. Signed-off-by: Sami Tolvanen Reviewed-by: Kees Cook --- arch/arm64/kvm/hyp/nvhe/Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile index a6707df4f6c0..fb24a0f022ad 100644 --- a/arch/arm64/kvm/hyp/nvhe/Makefile +++ b/arch/arm64/kvm/hyp/nvhe/Makefile @@ -75,9 +75,9 @@ quiet_cmd_hyprel = HYPREL $@ quiet_cmd_hypcopy = HYPCOPY $@ cmd_hypcopy = $(OBJCOPY) --prefix-symbols=__kvm_nvhe_ $< $@ -# Remove ftrace and Shadow Call Stack CFLAGS. -# This is equivalent to the 'notrace' and '__noscs' annotations. -KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS), $(KBUILD_CFLAGS)) +# Remove ftrace, Shadow Call Stack, and CFI CFLAGS. +# This is equivalent to the 'notrace', '__noscs', and '__nocfi' annotations. +KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS) $(CC_FLAGS_CFI), $(KBUILD_CFLAGS)) # KVM nVHE code is run at a different exception code with a different map, so # compiler instrumentation that inserts callbacks or checks into the code may -- 2.31.0.208.g409f899ff0-goog
[PATCH v5 18/18] arm64: allow CONFIG_CFI_CLANG to be selected
Select ARCH_SUPPORTS_CFI_CLANG to allow CFI to be enabled. Signed-off-by: Sami Tolvanen Reviewed-by: Kees Cook --- arch/arm64/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index e4e1b6550115..d7395772b6b8 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -75,6 +75,7 @@ config ARM64 select ARCH_SUPPORTS_SHADOW_CALL_STACK if CC_HAVE_SHADOW_CALL_STACK select ARCH_SUPPORTS_LTO_CLANG if CPU_LITTLE_ENDIAN select ARCH_SUPPORTS_LTO_CLANG_THIN + select ARCH_SUPPORTS_CFI_CLANG select ARCH_SUPPORTS_ATOMIC_RMW select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && (GCC_VERSION >= 5 || CC_IS_CLANG) select ARCH_SUPPORTS_NUMA_BALANCING -- 2.31.0.208.g409f899ff0-goog
[PATCH v5 13/18] arm64: use function_nocfi with __pa_symbol
With CONFIG_CFI_CLANG, the compiler replaces function address
references with the address of the function's CFI jump table
entry. This means that __pa_symbol(function) returns the physical
address of the jump table entry, which can lead to address space
confusion as the jump table points to the function's virtual
address. Therefore, use the function_nocfi() macro to ensure we are
always taking the address of the actual function instead.
Signed-off-by: Sami Tolvanen
Reviewed-by: Kees Cook
---
arch/arm64/include/asm/mmu_context.h | 2 +-
arch/arm64/kernel/acpi_parking_protocol.c | 3 ++-
arch/arm64/kernel/cpu-reset.h | 2 +-
arch/arm64/kernel/cpufeature.c| 2 +-
arch/arm64/kernel/psci.c | 3 ++-
arch/arm64/kernel/smp_spin_table.c| 3 ++-
6 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/include/asm/mmu_context.h
b/arch/arm64/include/asm/mmu_context.h
index bd02e99b1a4c..386b96400a57 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -140,7 +140,7 @@ static inline void cpu_replace_ttbr1(pgd_t *pgdp)
ttbr1 |= TTBR_CNP_BIT;
}
- replace_phys = (void *)__pa_symbol(idmap_cpu_replace_ttbr1);
+ replace_phys = (void
*)__pa_symbol(function_nocfi(idmap_cpu_replace_ttbr1));
cpu_install_idmap();
replace_phys(ttbr1);
diff --git a/arch/arm64/kernel/acpi_parking_protocol.c
b/arch/arm64/kernel/acpi_parking_protocol.c
index e7c941d8340d..bfeeb5319abf 100644
--- a/arch/arm64/kernel/acpi_parking_protocol.c
+++ b/arch/arm64/kernel/acpi_parking_protocol.c
@@ -99,7 +99,8 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
* that read this address need to convert this address to the
* Boot-Loader's endianness before jumping.
*/
- writeq_relaxed(__pa_symbol(secondary_entry), >entry_point);
+ writeq_relaxed(__pa_symbol(function_nocfi(secondary_entry)),
+ >entry_point);
writel_relaxed(cpu_entry->gic_cpu_id, >cpu_id);
arch_send_wakeup_ipi_mask(cpumask_of(cpu));
diff --git a/arch/arm64/kernel/cpu-reset.h b/arch/arm64/kernel/cpu-reset.h
index ed50e9587ad8..f3adc574f969 100644
--- a/arch/arm64/kernel/cpu-reset.h
+++ b/arch/arm64/kernel/cpu-reset.h
@@ -22,7 +22,7 @@ static inline void __noreturn cpu_soft_restart(unsigned long
entry,
unsigned long el2_switch = !is_kernel_in_hyp_mode() &&
is_hyp_mode_available();
- restart = (void *)__pa_symbol(__cpu_soft_restart);
+ restart = (void *)__pa_symbol(function_nocfi(__cpu_soft_restart));
cpu_install_idmap();
restart(el2_switch, entry, arg0, arg1, arg2);
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index e5281e1c8f1d..0b2e0d7b13ec 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1462,7 +1462,7 @@ kpti_install_ng_mappings(const struct
arm64_cpu_capabilities *__unused)
if (arm64_use_ng_mappings)
return;
- remap_fn = (void *)__pa_symbol(idmap_kpti_install_ng_mappings);
+ remap_fn = (void
*)__pa_symbol(function_nocfi(idmap_kpti_install_ng_mappings));
cpu_install_idmap();
remap_fn(cpu, num_online_cpus(), __pa_symbol(swapper_pg_dir));
diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
index 62d2bda7adb8..e74bcb57559b 100644
--- a/arch/arm64/kernel/psci.c
+++ b/arch/arm64/kernel/psci.c
@@ -38,7 +38,8 @@ static int __init cpu_psci_cpu_prepare(unsigned int cpu)
static int cpu_psci_cpu_boot(unsigned int cpu)
{
- int err = psci_ops.cpu_on(cpu_logical_map(cpu),
__pa_symbol(secondary_entry));
+ int err = psci_ops.cpu_on(cpu_logical_map(cpu),
+ __pa_symbol(function_nocfi(secondary_entry)));
if (err)
pr_err("failed to boot CPU%d (%d)\n", cpu, err);
diff --git a/arch/arm64/kernel/smp_spin_table.c
b/arch/arm64/kernel/smp_spin_table.c
index 056772c26098..4c4e36ded4aa 100644
--- a/arch/arm64/kernel/smp_spin_table.c
+++ b/arch/arm64/kernel/smp_spin_table.c
@@ -88,7 +88,8 @@ static int smp_spin_table_cpu_prepare(unsigned int cpu)
* boot-loader's endianness before jumping. This is mandated by
* the boot protocol.
*/
- writeq_relaxed(__pa_symbol(secondary_holding_pen), release_addr);
+ writeq_relaxed(__pa_symbol(function_nocfi(secondary_holding_pen)),
+ release_addr);
__flush_dcache_area((__force void *)release_addr,
sizeof(*release_addr));
--
2.31.0.208.g409f899ff0-goog
[PATCH v5 12/18] arm64: implement function_nocfi
With CONFIG_CFI_CLANG, the compiler replaces function addresses in
instrumented C code with jump table addresses. This change implements
the function_nocfi() macro, which returns the actual function address
instead.
Signed-off-by: Sami Tolvanen
Reviewed-by: Kees Cook
---
arch/arm64/include/asm/memory.h | 15 +++
1 file changed, 15 insertions(+)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 0aabc3be9a75..b55410afd3d1 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -321,6 +321,21 @@ static inline void *phys_to_virt(phys_addr_t x)
#define virt_to_pfn(x) __phys_to_pfn(__virt_to_phys((unsigned
long)(x)))
#define sym_to_pfn(x) __phys_to_pfn(__pa_symbol(x))
+#ifdef CONFIG_CFI_CLANG
+/*
+ * With CONFIG_CFI_CLANG, the compiler replaces function address
+ * references with the address of the function's CFI jump table
+ * entry. The function_nocfi macro always returns the address of the
+ * actual function instead.
+ */
+#define function_nocfi(x) ({ \
+ void *addr; \
+ asm("adrp %0, " __stringify(x) "\n\t" \
+ "add %0, %0, :lo12:" __stringify(x) : "=r" (addr));\
+ addr; \
+})
+#endif
+
/*
* virt_to_page(x)convert a _valid_ virtual address to struct page *
* virt_addr_valid(x) indicates whether a virtual address is valid
--
2.31.0.208.g409f899ff0-goog
[PATCH v5 14/18] arm64: add __nocfi to functions that jump to a physical address
Disable CFI checking for functions that switch to linear mapping and
make an indirect call to a physical address, since the compiler only
understands virtual addresses and the CFI check for such indirect calls
would always fail.
Signed-off-by: Sami Tolvanen
Reviewed-by: Kees Cook
---
arch/arm64/include/asm/mmu_context.h | 2 +-
arch/arm64/kernel/cpu-reset.h| 8
arch/arm64/kernel/cpufeature.c | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/include/asm/mmu_context.h
b/arch/arm64/include/asm/mmu_context.h
index 386b96400a57..d3cef9133539 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -119,7 +119,7 @@ static inline void cpu_install_idmap(void)
* Atomically replaces the active TTBR1_EL1 PGD with a new VA-compatible PGD,
* avoiding the possibility of conflicting TLB entries being allocated.
*/
-static inline void cpu_replace_ttbr1(pgd_t *pgdp)
+static inline void __nocfi cpu_replace_ttbr1(pgd_t *pgdp)
{
typedef void (ttbr_replace_func)(phys_addr_t);
extern ttbr_replace_func idmap_cpu_replace_ttbr1;
diff --git a/arch/arm64/kernel/cpu-reset.h b/arch/arm64/kernel/cpu-reset.h
index f3adc574f969..9a7b1262ef17 100644
--- a/arch/arm64/kernel/cpu-reset.h
+++ b/arch/arm64/kernel/cpu-reset.h
@@ -13,10 +13,10 @@
void __cpu_soft_restart(unsigned long el2_switch, unsigned long entry,
unsigned long arg0, unsigned long arg1, unsigned long arg2);
-static inline void __noreturn cpu_soft_restart(unsigned long entry,
- unsigned long arg0,
- unsigned long arg1,
- unsigned long arg2)
+static inline void __noreturn __nocfi cpu_soft_restart(unsigned long entry,
+ unsigned long arg0,
+ unsigned long arg1,
+ unsigned long arg2)
{
typeof(__cpu_soft_restart) *restart;
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 0b2e0d7b13ec..c2f94a5206e0 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1445,7 +1445,7 @@ static bool unmap_kernel_at_el0(const struct
arm64_cpu_capabilities *entry,
}
#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
-static void
+static void __nocfi
kpti_install_ng_mappings(const struct arm64_cpu_capabilities *__unused)
{
typedef void (kpti_remap_fn)(int, int, phys_addr_t);
--
2.31.0.208.g409f899ff0-goog
[PATCH v5 11/18] psci: use function_nocfi for cpu_resume
With CONFIG_CFI_CLANG, the compiler replaces function pointers with
jump table addresses, which results in __pa_symbol returning the
physical address of the jump table entry. As the jump table contains
an immediate jump to an EL1 virtual address, this typically won't
work as intended. Use function_nocfi to get the actual address of
cpu_resume.
Signed-off-by: Sami Tolvanen
Reviewed-by: Kees Cook
---
drivers/firmware/psci/psci.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index f5fc429cae3f..64344e84bd63 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -325,8 +325,9 @@ static int __init psci_features(u32 psci_func_id)
static int psci_suspend_finisher(unsigned long state)
{
u32 power_state = state;
+ phys_addr_t pa_cpu_resume = __pa_symbol(function_nocfi(cpu_resume));
- return psci_ops.cpu_suspend(power_state, __pa_symbol(cpu_resume));
+ return psci_ops.cpu_suspend(power_state, pa_cpu_resume);
}
int psci_cpu_suspend_enter(u32 state)
@@ -344,8 +345,10 @@ int psci_cpu_suspend_enter(u32 state)
static int psci_system_suspend(unsigned long unused)
{
+ phys_addr_t pa_cpu_resume = __pa_symbol(function_nocfi(cpu_resume));
+
return invoke_psci_fn(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND),
- __pa_symbol(cpu_resume), 0, 0);
+ pa_cpu_resume, 0, 0);
}
static int psci_system_suspend_enter(suspend_state_t state)
--
2.31.0.208.g409f899ff0-goog
[PATCH v5 10/18] lkdtm: use function_nocfi
To ensure we take the actual address of a function in kernel text,
use function_nocfi. Otherwise, with CONFIG_CFI_CLANG, the compiler
replaces the address with a pointer to the CFI jump table, which is
actually in the module when compiled with CONFIG_LKDTM=m.
Signed-off-by: Sami Tolvanen
Acked-by: Kees Cook
---
drivers/misc/lkdtm/usercopy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/misc/lkdtm/usercopy.c b/drivers/misc/lkdtm/usercopy.c
index 109e8d4302c1..15d220ef35a5 100644
--- a/drivers/misc/lkdtm/usercopy.c
+++ b/drivers/misc/lkdtm/usercopy.c
@@ -314,7 +314,7 @@ void lkdtm_USERCOPY_KERNEL(void)
pr_info("attempting bad copy_to_user from kernel text: %px\n",
vm_mmap);
- if (copy_to_user((void __user *)user_addr, vm_mmap,
+ if (copy_to_user((void __user *)user_addr, function_nocfi(vm_mmap),
unconst + PAGE_SIZE)) {
pr_warn("copy_to_user failed, but lacked Oops\n");
goto free_user;
--
2.31.0.208.g409f899ff0-goog
[PATCH v5 07/18] kallsyms: strip ThinLTO hashes from static functions
With CONFIG_CFI_CLANG and ThinLTO, Clang appends a hash to the names
of all static functions not marked __used. This can break userspace
tools that don't expect the function name to change, so strip out the
hash from the output.
Suggested-by: Jack Pham
Signed-off-by: Sami Tolvanen
Reviewed-by: Kees Cook
---
kernel/kallsyms.c | 55 ++-
1 file changed, 50 insertions(+), 5 deletions(-)
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 8043a90aa50e..c851ca0ed357 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -161,6 +161,27 @@ static unsigned long kallsyms_sym_address(int idx)
return kallsyms_relative_base - 1 - kallsyms_offsets[idx];
}
+#if defined(CONFIG_CFI_CLANG) && defined(CONFIG_LTO_CLANG_THIN)
+/*
+ * LLVM appends a hash to static function names when ThinLTO and CFI are
+ * both enabled, i.e. foo() becomes foo$707af9a22804d33c81801f27dcfe489b.
+ * This causes confusion and potentially breaks user space tools, so we
+ * strip the suffix from expanded symbol names.
+ */
+static inline bool cleanup_symbol_name(char *s)
+{
+ char *res;
+
+ res = strrchr(s, '$');
+ if (res)
+ *res = '\0';
+
+ return res != NULL;
+}
+#else
+static inline bool cleanup_symbol_name(char *s) { return false; }
+#endif
+
/* Lookup the address for this symbol. Returns 0 if not found. */
unsigned long kallsyms_lookup_name(const char *name)
{
@@ -173,6 +194,9 @@ unsigned long kallsyms_lookup_name(const char *name)
if (strcmp(namebuf, name) == 0)
return kallsyms_sym_address(i);
+
+ if (cleanup_symbol_name(namebuf) && strcmp(namebuf, name) == 0)
+ return kallsyms_sym_address(i);
}
return module_kallsyms_lookup_name(name);
}
@@ -303,7 +327,9 @@ const char *kallsyms_lookup(unsigned long addr,
namebuf, KSYM_NAME_LEN);
if (modname)
*modname = NULL;
- return namebuf;
+
+ ret = namebuf;
+ goto found;
}
/* See if it's in a module or a BPF JITed image. */
@@ -316,11 +342,16 @@ const char *kallsyms_lookup(unsigned long addr,
if (!ret)
ret = ftrace_mod_address_lookup(addr, symbolsize,
offset, modname, namebuf);
+
+found:
+ cleanup_symbol_name(namebuf);
return ret;
}
int lookup_symbol_name(unsigned long addr, char *symname)
{
+ int res;
+
symname[0] = '\0';
symname[KSYM_NAME_LEN - 1] = '\0';
@@ -331,15 +362,23 @@ int lookup_symbol_name(unsigned long addr, char *symname)
/* Grab name */
kallsyms_expand_symbol(get_symbol_offset(pos),
symname, KSYM_NAME_LEN);
- return 0;
+ goto found;
}
/* See if it's in a module. */
- return lookup_module_symbol_name(addr, symname);
+ res = lookup_module_symbol_name(addr, symname);
+ if (res)
+ return res;
+
+found:
+ cleanup_symbol_name(symname);
+ return 0;
}
int lookup_symbol_attrs(unsigned long addr, unsigned long *size,
unsigned long *offset, char *modname, char *name)
{
+ int res;
+
name[0] = '\0';
name[KSYM_NAME_LEN - 1] = '\0';
@@ -351,10 +390,16 @@ int lookup_symbol_attrs(unsigned long addr, unsigned long
*size,
kallsyms_expand_symbol(get_symbol_offset(pos),
name, KSYM_NAME_LEN);
modname[0] = '\0';
- return 0;
+ goto found;
}
/* See if it's in a module. */
- return lookup_module_symbol_attrs(addr, size, offset, modname, name);
+ res = lookup_module_symbol_attrs(addr, size, offset, modname, name);
+ if (res)
+ return res;
+
+found:
+ cleanup_symbol_name(name);
+ return 0;
}
/* Look up a kernel symbol and return it in a text buffer. */
--
2.31.0.208.g409f899ff0-goog
[PATCH v5 09/18] treewide: Change list_sort to use const pointers
list_sort() internally casts the comparison function passed to it
to a different type with constant struct list_head pointers, and
uses this pointer to call the functions, which trips indirect call
Control-Flow Integrity (CFI) checking.
Instead of removing the consts, this change defines the
list_cmp_func_t type and changes the comparison function types of
all list_sort() callers to use const pointers, thus avoiding type
mismatches.
Suggested-by: Nick Desaulniers
Signed-off-by: Sami Tolvanen
Reviewed-by: Nick Desaulniers
Tested-by: Nick Desaulniers
Reviewed-by: Christoph Hellwig
Reviewed-by: Kees Cook
---
arch/arm64/kvm/vgic/vgic-its.c | 8
arch/arm64/kvm/vgic/vgic.c | 3 ++-
block/blk-mq-sched.c| 3 ++-
block/blk-mq.c | 3 ++-
drivers/acpi/nfit/core.c| 3 ++-
drivers/acpi/numa/hmat.c| 3 ++-
drivers/clk/keystone/sci-clk.c | 4 ++--
drivers/gpu/drm/drm_modes.c | 3 ++-
drivers/gpu/drm/i915/gt/intel_engine_user.c | 3 ++-
drivers/gpu/drm/i915/gvt/debugfs.c | 2 +-
drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 3 ++-
drivers/gpu/drm/radeon/radeon_cs.c | 4 ++--
.../hw/usnic/usnic_uiom_interval_tree.c | 3 ++-
drivers/interconnect/qcom/bcm-voter.c | 2 +-
drivers/md/raid5.c | 3 ++-
drivers/misc/sram.c | 4 ++--
drivers/nvme/host/core.c| 3 ++-
.../pci/controller/cadence/pcie-cadence-host.c | 3 ++-
drivers/spi/spi-loopback-test.c | 3 ++-
fs/btrfs/raid56.c | 3 ++-
fs/btrfs/tree-log.c | 3 ++-
fs/btrfs/volumes.c | 3 ++-
fs/ext4/fsmap.c | 4 ++--
fs/gfs2/glock.c | 3 ++-
fs/gfs2/log.c | 2 +-
fs/gfs2/lops.c | 3 ++-
fs/iomap/buffered-io.c | 3 ++-
fs/ubifs/gc.c | 7 ---
fs/ubifs/replay.c | 4 ++--
fs/xfs/scrub/bitmap.c | 4 ++--
fs/xfs/xfs_bmap_item.c | 4 ++--
fs/xfs/xfs_buf.c| 6 +++---
fs/xfs/xfs_extent_busy.c| 4 ++--
fs/xfs/xfs_extent_busy.h| 3 ++-
fs/xfs/xfs_extfree_item.c | 4 ++--
fs/xfs/xfs_refcount_item.c | 4 ++--
fs/xfs/xfs_rmap_item.c | 4 ++--
include/linux/list_sort.h | 7 ---
lib/list_sort.c | 17 ++---
lib/test_list_sort.c| 3 ++-
net/tipc/name_table.c | 4 ++--
41 files changed, 90 insertions(+), 72 deletions(-)
diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index 40cbaca81333..b9518f94bd43 100644
--- a/arch/arm64/kvm/vgic/vgic-its.c
+++ b/arch/arm64/kvm/vgic/vgic-its.c
@@ -2190,8 +2190,8 @@ static int vgic_its_restore_ite(struct vgic_its *its, u32
event_id,
return offset;
}
-static int vgic_its_ite_cmp(void *priv, struct list_head *a,
- struct list_head *b)
+static int vgic_its_ite_cmp(void *priv, const struct list_head *a,
+ const struct list_head *b)
{
struct its_ite *itea = container_of(a, struct its_ite, ite_list);
struct its_ite *iteb = container_of(b, struct its_ite, ite_list);
@@ -2329,8 +2329,8 @@ static int vgic_its_restore_dte(struct vgic_its *its, u32
id,
return offset;
}
-static int vgic_its_device_cmp(void *priv, struct list_head *a,
- struct list_head *b)
+static int vgic_its_device_cmp(void *priv, const struct list_head *a,
+ const struct list_head *b)
{
struct its_device *deva = container_of(a, struct its_device, dev_list);
struct its_device *devb = container_of(b, struct its_device, dev_list);
diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
index 1c597c9885fa..15b666200f0b 100644
--- a/arch/arm64/kvm/vgic/vgic.c
+++ b/arch/arm64/kvm/vgic/vgic.c
@@ -255,7 +255,8 @@ static struct kvm_vcpu *vgic_target_oracle(struct vgic_irq
*irq)
* Return negative if "a" sorts before "b", 0 to preserve order, and positive
* to sort "b" before "a".
*/
-static int vgic_irq_cmp(void *priv, struct list_head *a, struct list_head *b)
+static int vgic_irq_cmp(void *priv, const struct list_head *a,
+ const struct list_head *b)
{
struct vgic_irq *irqa = container_of(a, struct vgic_irq, ap_list);
[PATCH v5 08/18] bpf: disable CFI in dispatcher functions
BPF dispatcher functions are patched at runtime to perform direct
instead of indirect calls. Disable CFI for the dispatcher functions to
avoid conflicts.
Signed-off-by: Sami Tolvanen
Reviewed-by: Kees Cook
---
include/linux/bpf.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 3625f019767d..2f46f98479e1 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -650,7 +650,7 @@ struct bpf_dispatcher {
struct bpf_ksym ksym;
};
-static __always_inline unsigned int bpf_dispatcher_nop_func(
+static __always_inline __nocfi unsigned int bpf_dispatcher_nop_func(
const void *ctx,
const struct bpf_insn *insnsi,
unsigned int (*bpf_func)(const void *,
@@ -678,7 +678,7 @@ void bpf_trampoline_put(struct bpf_trampoline *tr);
}
#define DEFINE_BPF_DISPATCHER(name)\
- noinline unsigned int bpf_dispatcher_##name##_func( \
+ noinline __nocfi unsigned int bpf_dispatcher_##name##_func( \
const void *ctx,\
const struct bpf_insn *insnsi, \
unsigned int (*bpf_func)(const void *, \
--
2.31.0.208.g409f899ff0-goog
[PATCH v5 05/18] workqueue: use WARN_ON_FUNCTION_MISMATCH
With CONFIG_CFI_CLANG, a callback function passed to __queue_delayed_work from a module points to a jump table entry defined in the module instead of the one used in the core kernel, which breaks function address equality in this check: WARN_ON_ONCE(timer->function != delayed_work_timer_fn); Use WARN_ON_FUNCTION_MISMATCH() instead to disable the warning when CFI and modules are both enabled. Signed-off-by: Sami Tolvanen Reviewed-by: Kees Cook --- kernel/workqueue.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 0d150da252e8..03fe07d2f39f 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -1630,7 +1630,7 @@ static void __queue_delayed_work(int cpu, struct workqueue_struct *wq, struct work_struct *work = >work; WARN_ON_ONCE(!wq); - WARN_ON_ONCE(timer->function != delayed_work_timer_fn); + WARN_ON_FUNCTION_MISMATCH(timer->function, delayed_work_timer_fn); WARN_ON_ONCE(timer_pending(timer)); WARN_ON_ONCE(!list_empty(>entry)); -- 2.31.0.208.g409f899ff0-goog
[PATCH v5 06/18] kthread: use WARN_ON_FUNCTION_MISMATCH
With CONFIG_CFI_CLANG, a callback function passed to __kthread_queue_delayed_work from a module points to a jump table entry defined in the module instead of the one used in the core kernel, which breaks function address equality in this check: WARN_ON_ONCE(timer->function != ktead_delayed_work_timer_fn); Use WARN_ON_FUNCTION_MISMATCH() instead to disable the warning when CFI and modules are both enabled. Signed-off-by: Sami Tolvanen Reviewed-by: Kees Cook --- kernel/kthread.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kernel/kthread.c b/kernel/kthread.c index 1578973c5740..a1972eba2917 100644 --- a/kernel/kthread.c +++ b/kernel/kthread.c @@ -963,7 +963,8 @@ static void __kthread_queue_delayed_work(struct kthread_worker *worker, struct timer_list *timer = >timer; struct kthread_work *work = >work; - WARN_ON_ONCE(timer->function != kthread_delayed_work_timer_fn); + WARN_ON_FUNCTION_MISMATCH(timer->function, + kthread_delayed_work_timer_fn); /* * If @delay is 0, queue @dwork->work immediately. This is for -- 2.31.0.208.g409f899ff0-goog
[PATCH v5 04/18] module: ensure __cfi_check alignment
CONFIG_CFI_CLANG_SHADOW assumes the __cfi_check() function is page
aligned and at the beginning of the .text section. While Clang would
normally align the function correctly, it fails to do so for modules
with no executable code.
This change ensures the correct __cfi_check() location and
alignment. It also discards the .eh_frame section, which Clang can
generate with certain sanitizers, such as CFI.
Link: https://bugs.llvm.org/show_bug.cgi?id=46293
Signed-off-by: Sami Tolvanen
Reviewed-by: Kees Cook
Acked-by: Jessica Yu
---
scripts/module.lds.S | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/scripts/module.lds.S b/scripts/module.lds.S
index 168cd27e6122..f8022b34e388 100644
--- a/scripts/module.lds.S
+++ b/scripts/module.lds.S
@@ -3,10 +3,20 @@
* Archs are free to supply their own linker scripts. ld will
* combine them automatically.
*/
+#ifdef CONFIG_CFI_CLANG
+# include
+# define ALIGN_CFI ALIGN(PAGE_SIZE)
+# define SANITIZER_DISCARDS*(.eh_frame)
+#else
+# define ALIGN_CFI
+# define SANITIZER_DISCARDS
+#endif
+
SECTIONS {
/DISCARD/ : {
*(.discard)
*(.discard.*)
+ SANITIZER_DISCARDS
}
__ksymtab 0 : { *(SORT(___ksymtab+*)) }
@@ -40,7 +50,14 @@ SECTIONS {
*(.rodata..L*)
}
- .text : { *(.text .text.[0-9a-zA-Z_]*) }
+ /*
+* With CONFIG_CFI_CLANG, we assume __cfi_check is at the beginning
+* of the .text section, and is aligned to PAGE_SIZE.
+*/
+ .text : ALIGN_CFI {
+ *(.text.__cfi_check)
+ *(.text .text.[0-9a-zA-Z_]* .text..L.cfi*)
+ }
}
/* bring in arch-specific sections */
--
2.31.0.208.g409f899ff0-goog
[PATCH v5 03/18] mm: add generic function_nocfi macro
With CONFIG_CFI_CLANG, the compiler replaces function addresses in instrumented C code with jump table addresses. This means that __pa_symbol(function) returns the physical address of the jump table entry instead of the actual function, which may not work as the jump table code will immediately jump to a virtual address that may not be mapped. To avoid this address space confusion, this change adds a generic definition for function_nocfi(), which architectures that support CFI can override. The typical implementation of would use inline assembly to take the function address, which avoids compiler instrumentation. Signed-off-by: Sami Tolvanen Reviewed-by: Kees Cook --- include/linux/mm.h | 10 ++ 1 file changed, 10 insertions(+) diff --git a/include/linux/mm.h b/include/linux/mm.h index 8ba434287387..22cce9c7dd05 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -124,6 +124,16 @@ extern int mmap_rnd_compat_bits __read_mostly; #define lm_alias(x)__va(__pa_symbol(x)) #endif +/* + * With CONFIG_CFI_CLANG, the compiler replaces function addresses in + * instrumented C code with jump table addresses. Architectures that + * support CFI can define this macro to return the actual function address + * when needed. + */ +#ifndef function_nocfi +#define function_nocfi(x) (x) +#endif + /* * To prevent common memory management code establishing * a zero page mapping on a read fault. -- 2.31.0.208.g409f899ff0-goog
[PATCH v5 02/18] cfi: add __cficanonical
With CONFIG_CFI_CLANG, the compiler replaces a function address taken
in C code with the address of a local jump table entry, which passes
runtime indirect call checks. However, the compiler won't replace
addresses taken in assembly code, which will result in a CFI failure
if we later jump to such an address in instrumented C code. The code
generated for the non-canonical jump table looks this:
: /* In C, points here */
jmp noncanonical
...
:/* function body */
...
This change adds the __cficanonical attribute, which tells the
compiler to use a canonical jump table for the function instead. This
means the compiler will rename the actual function to .cfi
and points the original symbol to the jump table entry instead:
: /* jump table entry */
jmp canonical.cfi
...
: /* function body */
...
As a result, the address taken in assembly, or other non-instrumented
code always points to the jump table and therefore, can be used for
indirect calls in instrumented code without tripping CFI checks.
Signed-off-by: Sami Tolvanen
Reviewed-by: Kees Cook
Acked-by: Bjorn Helgaas# pci.h
---
include/linux/compiler-clang.h | 1 +
include/linux/compiler_types.h | 4
include/linux/init.h | 4 ++--
include/linux/pci.h| 4 ++--
4 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index 6de9d0c9377e..adbe76b203e2 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -63,3 +63,4 @@
#endif
#define __nocfi__attribute__((__no_sanitize__("cfi")))
+#define __cficanonical __attribute__((__cfi_canonical_jump_table__))
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 796935a37e37..d29bda7f6ebd 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -246,6 +246,10 @@ struct ftrace_likely_data {
# define __nocfi
#endif
+#ifndef __cficanonical
+# define __cficanonical
+#endif
+
#ifndef asm_volatile_goto
#define asm_volatile_goto(x...) asm goto(x)
#endif
diff --git a/include/linux/init.h b/include/linux/init.h
index b3ea15348fbd..045ad1650ed1 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -220,8 +220,8 @@ extern bool initcall_debug;
__initcall_name(initstub, __iid, id)
#define __define_initcall_stub(__stub, fn) \
- int __init __stub(void);\
- int __init __stub(void) \
+ int __init __cficanonical __stub(void); \
+ int __init __cficanonical __stub(void) \
{ \
return fn();\
} \
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 86c799c97b77..39684b72db91 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1944,8 +1944,8 @@ enum pci_fixup_pass {
#ifdef CONFIG_LTO_CLANG
#define __DECLARE_PCI_FIXUP_SECTION(sec, name, vendor, device, class, \
class_shift, hook, stub) \
- void stub(struct pci_dev *dev); \
- void stub(struct pci_dev *dev) \
+ void __cficanonical stub(struct pci_dev *dev); \
+ void __cficanonical stub(struct pci_dev *dev) \
{ \
hook(dev); \
} \
--
2.31.0.208.g409f899ff0-goog
[PATCH v5 01/18] add support for Clang CFI
This change adds support for Clang’s forward-edge Control Flow Integrity (CFI) checking. With CONFIG_CFI_CLANG, the compiler injects a runtime check before each indirect function call to ensure the target is a valid function with the correct static type. This restricts possible call targets and makes it more difficult for an attacker to exploit bugs that allow the modification of stored function pointers. For more details, see: https://clang.llvm.org/docs/ControlFlowIntegrity.html Clang requires CONFIG_LTO_CLANG to be enabled with CFI to gain visibility to possible call targets. Kernel modules are supported with Clang’s cross-DSO CFI mode, which allows checking between independently compiled components. With CFI enabled, the compiler injects a __cfi_check() function into the kernel and each module for validating local call targets. For cross-module calls that cannot be validated locally, the compiler calls the global __cfi_slowpath_diag() function, which determines the target module and calls the correct __cfi_check() function. This patch includes a slowpath implementation that uses __module_address() to resolve call targets, and with CONFIG_CFI_CLANG_SHADOW enabled, a shadow map that speeds up module look-ups by ~3x. Clang implements indirect call checking using jump tables and offers two methods of generating them. With canonical jump tables, the compiler renames each address-taken function to .cfi and points the original symbol to a jump table entry, which passes __cfi_check() validation. This isn’t compatible with stand-alone assembly code, which the compiler doesn’t instrument, and would result in indirect calls to assembly code to fail. Therefore, we default to using non-canonical jump tables instead, where the compiler generates a local jump table entry .cfi_jt for each address-taken function, and replaces all references to the function with the address of the jump table entry. Note that because non-canonical jump table addresses are local to each component, they break cross-module function address equality. Specifically, the address of a global function will be different in each module, as it's replaced with the address of a local jump table entry. If this address is passed to a different module, it won’t match the address of the same function taken there. This may break code that relies on comparing addresses passed from other components. CFI checking can be disabled in a function with the __nocfi attribute. Additionally, CFI can be disabled for an entire compilation unit by filtering out CC_FLAGS_CFI. By default, CFI failures result in a kernel panic to stop a potential exploit. CONFIG_CFI_PERMISSIVE enables a permissive mode, where the kernel prints out a rate-limited warning instead, and allows execution to continue. This option is helpful for locating type mismatches, but should only be enabled during development. Signed-off-by: Sami Tolvanen Reviewed-by: Kees Cook --- Makefile | 17 ++ arch/Kconfig | 45 include/asm-generic/bug.h | 16 ++ include/asm-generic/vmlinux.lds.h | 20 +- include/linux/cfi.h | 41 include/linux/compiler-clang.h| 2 + include/linux/compiler_types.h| 4 + include/linux/init.h | 2 +- include/linux/module.h| 13 +- init/Kconfig | 2 +- kernel/Makefile | 4 + kernel/cfi.c | 329 ++ kernel/module.c | 43 scripts/Makefile.modfinal | 2 +- 14 files changed, 534 insertions(+), 6 deletions(-) create mode 100644 include/linux/cfi.h create mode 100644 kernel/cfi.c diff --git a/Makefile b/Makefile index 73add16f9898..4a9c1ff420b6 100644 --- a/Makefile +++ b/Makefile @@ -920,6 +920,23 @@ KBUILD_AFLAGS += -fno-lto export CC_FLAGS_LTO endif +ifdef CONFIG_CFI_CLANG +CC_FLAGS_CFI := -fsanitize=cfi \ + -fsanitize-cfi-cross-dso \ + -fno-sanitize-cfi-canonical-jump-tables \ + -fno-sanitize-trap=cfi \ + -fno-sanitize-blacklist + +ifdef CONFIG_CFI_PERMISSIVE +CC_FLAGS_CFI += -fsanitize-recover=cfi +endif + +# If LTO flags are filtered out, we must also filter out CFI. +CC_FLAGS_LTO += $(CC_FLAGS_CFI) +KBUILD_CFLAGS += $(CC_FLAGS_CFI) +export CC_FLAGS_CFI +endif + ifdef CONFIG_DEBUG_FORCE_FUNCTION_ALIGN_32B KBUILD_CFLAGS += -falign-functions=32 endif diff --git a/arch/Kconfig b/arch/Kconfig index ecfd3520b676..f6a85ba6cba2 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -692,6 +692,51 @@ config LTO_CLANG_THIN If unsure, say Y. endchoice +config ARCH_SUPPORTS_CFI_CLANG + bool + help + An architecture should select this option if it can support Clang's + Control-Flow Integrity (CFI) checking. + +config CFI_CLANG + bool "Use Clang's Control Flow Integrity (CFI)" + depends on
[PATCH v5 00/18] Add support for Clang CFI
This series adds support for Clang's Control-Flow Integrity (CFI) checking. With CFI, the compiler injects a runtime check before each indirect function call to ensure the target is a valid function with the correct static type. This restricts possible call targets and makes it more difficult for an attacker to exploit bugs that allow the modification of stored function pointers. For more details, see: https://clang.llvm.org/docs/ControlFlowIntegrity.html The first patch contains build system changes and error handling, and implements support for cross-module indirect call checking. The remaining patches address issues caused by the compiler instrumentation. These include fixing known type mismatches, as well as issues with address space confusion and cross-module function address equality. These patches add support only for arm64, but I'll post patches also for x86_64 after we address the remaining issues there, including objtool support. You can also pull this series from https://github.com/samitolvanen/linux.git cfi-v5 --- Changes in v5: - Changed module.lds.S to only include when CFI is enabled to fix the MIPS build. - Added a patch that fixes dynamic ftrace with CFI on arm64. Changes in v4: - Per Mark's suggestion, dropped __pa_function() and renamed __va_function() to function_nocfi(). - Added a comment to function_nocfi() to explain what it does. - Updated the psci patch to use an intermediate variable for the physical address for clarity. Changes in v3: - Added a patch to change list_sort() callers treewide to use const pointers instead of simply removing the internal casts. - Changed cleanup_symbol_name() to return bool. - Changed module.lds.S to drop the .eh_frame section only with CONFIG_CFI_CLANG. - Switched to synchronize_rcu() in update_shadow(). Changes in v2: - Fixed .text merging in module.lds.S. - Added WARN_ON_FUNCTION_MISMATCH() and changed kernel/thread.c and kernel/workqueue.c to use the macro instead. Sami Tolvanen (18): add support for Clang CFI cfi: add __cficanonical mm: add generic function_nocfi macro module: ensure __cfi_check alignment workqueue: use WARN_ON_FUNCTION_MISMATCH kthread: use WARN_ON_FUNCTION_MISMATCH kallsyms: strip ThinLTO hashes from static functions bpf: disable CFI in dispatcher functions treewide: Change list_sort to use const pointers lkdtm: use function_nocfi psci: use function_nocfi for cpu_resume arm64: implement function_nocfi arm64: use function_nocfi with __pa_symbol arm64: add __nocfi to functions that jump to a physical address arm64: add __nocfi to __apply_alternatives arm64: ftrace: use function_nocfi for ftrace_call KVM: arm64: Disable CFI for nVHE arm64: allow CONFIG_CFI_CLANG to be selected Makefile | 17 + arch/Kconfig | 45 +++ arch/arm64/Kconfig| 1 + arch/arm64/include/asm/memory.h | 15 + arch/arm64/include/asm/mmu_context.h | 4 +- arch/arm64/kernel/acpi_parking_protocol.c | 3 +- arch/arm64/kernel/alternative.c | 4 +- arch/arm64/kernel/cpu-reset.h | 10 +- arch/arm64/kernel/cpufeature.c| 4 +- arch/arm64/kernel/ftrace.c| 2 +- arch/arm64/kernel/psci.c | 3 +- arch/arm64/kernel/smp_spin_table.c| 3 +- arch/arm64/kvm/hyp/nvhe/Makefile | 6 +- arch/arm64/kvm/vgic/vgic-its.c| 8 +- arch/arm64/kvm/vgic/vgic.c| 3 +- block/blk-mq-sched.c | 3 +- block/blk-mq.c| 3 +- drivers/acpi/nfit/core.c | 3 +- drivers/acpi/numa/hmat.c | 3 +- drivers/clk/keystone/sci-clk.c| 4 +- drivers/firmware/psci/psci.c | 7 +- drivers/gpu/drm/drm_modes.c | 3 +- drivers/gpu/drm/i915/gt/intel_engine_user.c | 3 +- drivers/gpu/drm/i915/gvt/debugfs.c| 2 +- drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 3 +- drivers/gpu/drm/radeon/radeon_cs.c| 4 +- .../hw/usnic/usnic_uiom_interval_tree.c | 3 +- drivers/interconnect/qcom/bcm-voter.c | 2 +- drivers/md/raid5.c| 3 +- drivers/misc/lkdtm/usercopy.c | 2 +- drivers/misc/sram.c | 4 +- drivers/nvme/host/core.c | 3 +- .../controller/cadence/pcie-cadence-host.c| 3 +- drivers/spi/spi-loopback-test.c | 3 +- fs/btrfs/raid56.c | 3 +- fs/btrfs/tree-log.c | 3 +- fs/btrfs/volumes.c| 3 +- fs/ext4/fsmap.c | 4 +- fs/gfs2/glock.c | 3 +- fs/gfs2/log.c
[PATCH v4 16/17] KVM: arm64: Disable CFI for nVHE
Disable CFI for the nVHE code to avoid address space confusion. Signed-off-by: Sami Tolvanen Reviewed-by: Kees Cook --- arch/arm64/kvm/hyp/nvhe/Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile index a6707df4f6c0..fb24a0f022ad 100644 --- a/arch/arm64/kvm/hyp/nvhe/Makefile +++ b/arch/arm64/kvm/hyp/nvhe/Makefile @@ -75,9 +75,9 @@ quiet_cmd_hyprel = HYPREL $@ quiet_cmd_hypcopy = HYPCOPY $@ cmd_hypcopy = $(OBJCOPY) --prefix-symbols=__kvm_nvhe_ $< $@ -# Remove ftrace and Shadow Call Stack CFLAGS. -# This is equivalent to the 'notrace' and '__noscs' annotations. -KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS), $(KBUILD_CFLAGS)) +# Remove ftrace, Shadow Call Stack, and CFI CFLAGS. +# This is equivalent to the 'notrace', '__noscs', and '__nocfi' annotations. +KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS) $(CC_FLAGS_CFI), $(KBUILD_CFLAGS)) # KVM nVHE code is run at a different exception code with a different map, so # compiler instrumentation that inserts callbacks or checks into the code may -- 2.31.0.291.g576ba9dcdaf-goog
[PATCH v4 15/17] arm64: add __nocfi to __apply_alternatives
__apply_alternatives makes indirect calls to functions whose address
is taken in assembly code using the alternative_cb macro. With
non-canonical CFI, the compiler won't replace these function
references with the jump table addresses, which trips CFI. Disable CFI
checking in the function to work around the issue.
Signed-off-by: Sami Tolvanen
Reviewed-by: Kees Cook
---
arch/arm64/kernel/alternative.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/alternative.c b/arch/arm64/kernel/alternative.c
index 1184c44ea2c7..abc84636af07 100644
--- a/arch/arm64/kernel/alternative.c
+++ b/arch/arm64/kernel/alternative.c
@@ -133,8 +133,8 @@ static void clean_dcache_range_nopatch(u64 start, u64 end)
} while (cur += d_size, cur < end);
}
-static void __apply_alternatives(void *alt_region, bool is_module,
-unsigned long *feature_mask)
+static void __nocfi __apply_alternatives(void *alt_region, bool is_module,
+unsigned long *feature_mask)
{
struct alt_instr *alt;
struct alt_region *region = alt_region;
--
2.31.0.291.g576ba9dcdaf-goog
[PATCH v4 17/17] arm64: allow CONFIG_CFI_CLANG to be selected
Select ARCH_SUPPORTS_CFI_CLANG to allow CFI to be enabled. Signed-off-by: Sami Tolvanen Reviewed-by: Kees Cook --- arch/arm64/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index e4e1b6550115..d7395772b6b8 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -75,6 +75,7 @@ config ARM64 select ARCH_SUPPORTS_SHADOW_CALL_STACK if CC_HAVE_SHADOW_CALL_STACK select ARCH_SUPPORTS_LTO_CLANG if CPU_LITTLE_ENDIAN select ARCH_SUPPORTS_LTO_CLANG_THIN + select ARCH_SUPPORTS_CFI_CLANG select ARCH_SUPPORTS_ATOMIC_RMW select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && (GCC_VERSION >= 5 || CC_IS_CLANG) select ARCH_SUPPORTS_NUMA_BALANCING -- 2.31.0.291.g576ba9dcdaf-goog
[PATCH v4 10/17] lkdtm: use function_nocfi
To ensure we take the actual address of a function in kernel text,
use function_nocfi. Otherwise, with CONFIG_CFI_CLANG, the compiler
replaces the address with a pointer to the CFI jump table, which is
actually in the module when compiled with CONFIG_LKDTM=m.
Signed-off-by: Sami Tolvanen
Acked-by: Kees Cook
---
drivers/misc/lkdtm/usercopy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/misc/lkdtm/usercopy.c b/drivers/misc/lkdtm/usercopy.c
index 109e8d4302c1..15d220ef35a5 100644
--- a/drivers/misc/lkdtm/usercopy.c
+++ b/drivers/misc/lkdtm/usercopy.c
@@ -314,7 +314,7 @@ void lkdtm_USERCOPY_KERNEL(void)
pr_info("attempting bad copy_to_user from kernel text: %px\n",
vm_mmap);
- if (copy_to_user((void __user *)user_addr, vm_mmap,
+ if (copy_to_user((void __user *)user_addr, function_nocfi(vm_mmap),
unconst + PAGE_SIZE)) {
pr_warn("copy_to_user failed, but lacked Oops\n");
goto free_user;
--
2.31.0.291.g576ba9dcdaf-goog
[PATCH v4 14/17] arm64: add __nocfi to functions that jump to a physical address
Disable CFI checking for functions that switch to linear mapping and
make an indirect call to a physical address, since the compiler only
understands virtual addresses and the CFI check for such indirect calls
would always fail.
Signed-off-by: Sami Tolvanen
Reviewed-by: Kees Cook
---
arch/arm64/include/asm/mmu_context.h | 2 +-
arch/arm64/kernel/cpu-reset.h| 8
arch/arm64/kernel/cpufeature.c | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/include/asm/mmu_context.h
b/arch/arm64/include/asm/mmu_context.h
index 386b96400a57..d3cef9133539 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -119,7 +119,7 @@ static inline void cpu_install_idmap(void)
* Atomically replaces the active TTBR1_EL1 PGD with a new VA-compatible PGD,
* avoiding the possibility of conflicting TLB entries being allocated.
*/
-static inline void cpu_replace_ttbr1(pgd_t *pgdp)
+static inline void __nocfi cpu_replace_ttbr1(pgd_t *pgdp)
{
typedef void (ttbr_replace_func)(phys_addr_t);
extern ttbr_replace_func idmap_cpu_replace_ttbr1;
diff --git a/arch/arm64/kernel/cpu-reset.h b/arch/arm64/kernel/cpu-reset.h
index f3adc574f969..9a7b1262ef17 100644
--- a/arch/arm64/kernel/cpu-reset.h
+++ b/arch/arm64/kernel/cpu-reset.h
@@ -13,10 +13,10 @@
void __cpu_soft_restart(unsigned long el2_switch, unsigned long entry,
unsigned long arg0, unsigned long arg1, unsigned long arg2);
-static inline void __noreturn cpu_soft_restart(unsigned long entry,
- unsigned long arg0,
- unsigned long arg1,
- unsigned long arg2)
+static inline void __noreturn __nocfi cpu_soft_restart(unsigned long entry,
+ unsigned long arg0,
+ unsigned long arg1,
+ unsigned long arg2)
{
typeof(__cpu_soft_restart) *restart;
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index ac616c59ae92..1cd7877deada 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1446,7 +1446,7 @@ static bool unmap_kernel_at_el0(const struct
arm64_cpu_capabilities *entry,
}
#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
-static void
+static void __nocfi
kpti_install_ng_mappings(const struct arm64_cpu_capabilities *__unused)
{
typedef void (kpti_remap_fn)(int, int, phys_addr_t);
--
2.31.0.291.g576ba9dcdaf-goog
[PATCH v4 12/17] arm64: implement function_nocfi
With CONFIG_CFI_CLANG, the compiler replaces function addresses in
instrumented C code with jump table addresses. This change implements
the function_nocfi() macro, which returns the actual function address
instead.
Signed-off-by: Sami Tolvanen
Reviewed-by: Kees Cook
---
arch/arm64/include/asm/memory.h | 15 +++
1 file changed, 15 insertions(+)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 0aabc3be9a75..b55410afd3d1 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -321,6 +321,21 @@ static inline void *phys_to_virt(phys_addr_t x)
#define virt_to_pfn(x) __phys_to_pfn(__virt_to_phys((unsigned
long)(x)))
#define sym_to_pfn(x) __phys_to_pfn(__pa_symbol(x))
+#ifdef CONFIG_CFI_CLANG
+/*
+ * With CONFIG_CFI_CLANG, the compiler replaces function address
+ * references with the address of the function's CFI jump table
+ * entry. The function_nocfi macro always returns the address of the
+ * actual function instead.
+ */
+#define function_nocfi(x) ({ \
+ void *addr; \
+ asm("adrp %0, " __stringify(x) "\n\t" \
+ "add %0, %0, :lo12:" __stringify(x) : "=r" (addr));\
+ addr; \
+})
+#endif
+
/*
* virt_to_page(x)convert a _valid_ virtual address to struct page *
* virt_addr_valid(x) indicates whether a virtual address is valid
--
2.31.0.291.g576ba9dcdaf-goog
[PATCH v4 11/17] psci: use function_nocfi for cpu_resume
With CONFIG_CFI_CLANG, the compiler replaces function pointers with
jump table addresses, which results in __pa_symbol returning the
physical address of the jump table entry. As the jump table contains
an immediate jump to an EL1 virtual address, this typically won't
work as intended. Use function_nocfi to get the actual address of
cpu_resume.
Signed-off-by: Sami Tolvanen
Reviewed-by: Kees Cook
---
drivers/firmware/psci/psci.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index f5fc429cae3f..64344e84bd63 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -325,8 +325,9 @@ static int __init psci_features(u32 psci_func_id)
static int psci_suspend_finisher(unsigned long state)
{
u32 power_state = state;
+ phys_addr_t pa_cpu_resume = __pa_symbol(function_nocfi(cpu_resume));
- return psci_ops.cpu_suspend(power_state, __pa_symbol(cpu_resume));
+ return psci_ops.cpu_suspend(power_state, pa_cpu_resume);
}
int psci_cpu_suspend_enter(u32 state)
@@ -344,8 +345,10 @@ int psci_cpu_suspend_enter(u32 state)
static int psci_system_suspend(unsigned long unused)
{
+ phys_addr_t pa_cpu_resume = __pa_symbol(function_nocfi(cpu_resume));
+
return invoke_psci_fn(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND),
- __pa_symbol(cpu_resume), 0, 0);
+ pa_cpu_resume, 0, 0);
}
static int psci_system_suspend_enter(suspend_state_t state)
--
2.31.0.291.g576ba9dcdaf-goog
[PATCH v4 13/17] arm64: use function_nocfi with __pa_symbol
With CONFIG_CFI_CLANG, the compiler replaces function address
references with the address of the function's CFI jump table
entry. This means that __pa_symbol(function) returns the physical
address of the jump table entry, which can lead to address space
confusion as the jump table points to the function's virtual
address. Therefore, use the function_nocfi() macro to ensure we are
always taking the address of the actual function instead.
Signed-off-by: Sami Tolvanen
Reviewed-by: Kees Cook
---
arch/arm64/include/asm/mmu_context.h | 2 +-
arch/arm64/kernel/acpi_parking_protocol.c | 3 ++-
arch/arm64/kernel/cpu-reset.h | 2 +-
arch/arm64/kernel/cpufeature.c| 2 +-
arch/arm64/kernel/psci.c | 3 ++-
arch/arm64/kernel/smp_spin_table.c| 3 ++-
6 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/include/asm/mmu_context.h
b/arch/arm64/include/asm/mmu_context.h
index bd02e99b1a4c..386b96400a57 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -140,7 +140,7 @@ static inline void cpu_replace_ttbr1(pgd_t *pgdp)
ttbr1 |= TTBR_CNP_BIT;
}
- replace_phys = (void *)__pa_symbol(idmap_cpu_replace_ttbr1);
+ replace_phys = (void
*)__pa_symbol(function_nocfi(idmap_cpu_replace_ttbr1));
cpu_install_idmap();
replace_phys(ttbr1);
diff --git a/arch/arm64/kernel/acpi_parking_protocol.c
b/arch/arm64/kernel/acpi_parking_protocol.c
index e7c941d8340d..bfeeb5319abf 100644
--- a/arch/arm64/kernel/acpi_parking_protocol.c
+++ b/arch/arm64/kernel/acpi_parking_protocol.c
@@ -99,7 +99,8 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
* that read this address need to convert this address to the
* Boot-Loader's endianness before jumping.
*/
- writeq_relaxed(__pa_symbol(secondary_entry), >entry_point);
+ writeq_relaxed(__pa_symbol(function_nocfi(secondary_entry)),
+ >entry_point);
writel_relaxed(cpu_entry->gic_cpu_id, >cpu_id);
arch_send_wakeup_ipi_mask(cpumask_of(cpu));
diff --git a/arch/arm64/kernel/cpu-reset.h b/arch/arm64/kernel/cpu-reset.h
index ed50e9587ad8..f3adc574f969 100644
--- a/arch/arm64/kernel/cpu-reset.h
+++ b/arch/arm64/kernel/cpu-reset.h
@@ -22,7 +22,7 @@ static inline void __noreturn cpu_soft_restart(unsigned long
entry,
unsigned long el2_switch = !is_kernel_in_hyp_mode() &&
is_hyp_mode_available();
- restart = (void *)__pa_symbol(__cpu_soft_restart);
+ restart = (void *)__pa_symbol(function_nocfi(__cpu_soft_restart));
cpu_install_idmap();
restart(el2_switch, entry, arg0, arg1, arg2);
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 2a5d9854d664..ac616c59ae92 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1463,7 +1463,7 @@ kpti_install_ng_mappings(const struct
arm64_cpu_capabilities *__unused)
if (arm64_use_ng_mappings)
return;
- remap_fn = (void *)__pa_symbol(idmap_kpti_install_ng_mappings);
+ remap_fn = (void
*)__pa_symbol(function_nocfi(idmap_kpti_install_ng_mappings));
cpu_install_idmap();
remap_fn(cpu, num_online_cpus(), __pa_symbol(swapper_pg_dir));
diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
index 62d2bda7adb8..e74bcb57559b 100644
--- a/arch/arm64/kernel/psci.c
+++ b/arch/arm64/kernel/psci.c
@@ -38,7 +38,8 @@ static int __init cpu_psci_cpu_prepare(unsigned int cpu)
static int cpu_psci_cpu_boot(unsigned int cpu)
{
- int err = psci_ops.cpu_on(cpu_logical_map(cpu),
__pa_symbol(secondary_entry));
+ int err = psci_ops.cpu_on(cpu_logical_map(cpu),
+ __pa_symbol(function_nocfi(secondary_entry)));
if (err)
pr_err("failed to boot CPU%d (%d)\n", cpu, err);
diff --git a/arch/arm64/kernel/smp_spin_table.c
b/arch/arm64/kernel/smp_spin_table.c
index 056772c26098..4c4e36ded4aa 100644
--- a/arch/arm64/kernel/smp_spin_table.c
+++ b/arch/arm64/kernel/smp_spin_table.c
@@ -88,7 +88,8 @@ static int smp_spin_table_cpu_prepare(unsigned int cpu)
* boot-loader's endianness before jumping. This is mandated by
* the boot protocol.
*/
- writeq_relaxed(__pa_symbol(secondary_holding_pen), release_addr);
+ writeq_relaxed(__pa_symbol(function_nocfi(secondary_holding_pen)),
+ release_addr);
__flush_dcache_area((__force void *)release_addr,
sizeof(*release_addr));
--
2.31.0.291.g576ba9dcdaf-goog
[PATCH v4 07/17] kallsyms: strip ThinLTO hashes from static functions
With CONFIG_CFI_CLANG and ThinLTO, Clang appends a hash to the names
of all static functions not marked __used. This can break userspace
tools that don't expect the function name to change, so strip out the
hash from the output.
Suggested-by: Jack Pham
Signed-off-by: Sami Tolvanen
Reviewed-by: Kees Cook
---
kernel/kallsyms.c | 55 ++-
1 file changed, 50 insertions(+), 5 deletions(-)
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 8043a90aa50e..c851ca0ed357 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -161,6 +161,27 @@ static unsigned long kallsyms_sym_address(int idx)
return kallsyms_relative_base - 1 - kallsyms_offsets[idx];
}
+#if defined(CONFIG_CFI_CLANG) && defined(CONFIG_LTO_CLANG_THIN)
+/*
+ * LLVM appends a hash to static function names when ThinLTO and CFI are
+ * both enabled, i.e. foo() becomes foo$707af9a22804d33c81801f27dcfe489b.
+ * This causes confusion and potentially breaks user space tools, so we
+ * strip the suffix from expanded symbol names.
+ */
+static inline bool cleanup_symbol_name(char *s)
+{
+ char *res;
+
+ res = strrchr(s, '$');
+ if (res)
+ *res = '\0';
+
+ return res != NULL;
+}
+#else
+static inline bool cleanup_symbol_name(char *s) { return false; }
+#endif
+
/* Lookup the address for this symbol. Returns 0 if not found. */
unsigned long kallsyms_lookup_name(const char *name)
{
@@ -173,6 +194,9 @@ unsigned long kallsyms_lookup_name(const char *name)
if (strcmp(namebuf, name) == 0)
return kallsyms_sym_address(i);
+
+ if (cleanup_symbol_name(namebuf) && strcmp(namebuf, name) == 0)
+ return kallsyms_sym_address(i);
}
return module_kallsyms_lookup_name(name);
}
@@ -303,7 +327,9 @@ const char *kallsyms_lookup(unsigned long addr,
namebuf, KSYM_NAME_LEN);
if (modname)
*modname = NULL;
- return namebuf;
+
+ ret = namebuf;
+ goto found;
}
/* See if it's in a module or a BPF JITed image. */
@@ -316,11 +342,16 @@ const char *kallsyms_lookup(unsigned long addr,
if (!ret)
ret = ftrace_mod_address_lookup(addr, symbolsize,
offset, modname, namebuf);
+
+found:
+ cleanup_symbol_name(namebuf);
return ret;
}
int lookup_symbol_name(unsigned long addr, char *symname)
{
+ int res;
+
symname[0] = '\0';
symname[KSYM_NAME_LEN - 1] = '\0';
@@ -331,15 +362,23 @@ int lookup_symbol_name(unsigned long addr, char *symname)
/* Grab name */
kallsyms_expand_symbol(get_symbol_offset(pos),
symname, KSYM_NAME_LEN);
- return 0;
+ goto found;
}
/* See if it's in a module. */
- return lookup_module_symbol_name(addr, symname);
+ res = lookup_module_symbol_name(addr, symname);
+ if (res)
+ return res;
+
+found:
+ cleanup_symbol_name(symname);
+ return 0;
}
int lookup_symbol_attrs(unsigned long addr, unsigned long *size,
unsigned long *offset, char *modname, char *name)
{
+ int res;
+
name[0] = '\0';
name[KSYM_NAME_LEN - 1] = '\0';
@@ -351,10 +390,16 @@ int lookup_symbol_attrs(unsigned long addr, unsigned long
*size,
kallsyms_expand_symbol(get_symbol_offset(pos),
name, KSYM_NAME_LEN);
modname[0] = '\0';
- return 0;
+ goto found;
}
/* See if it's in a module. */
- return lookup_module_symbol_attrs(addr, size, offset, modname, name);
+ res = lookup_module_symbol_attrs(addr, size, offset, modname, name);
+ if (res)
+ return res;
+
+found:
+ cleanup_symbol_name(name);
+ return 0;
}
/* Look up a kernel symbol and return it in a text buffer. */
--
2.31.0.291.g576ba9dcdaf-goog
[PATCH v4 09/17] treewide: Change list_sort to use const pointers
list_sort() internally casts the comparison function passed to it
to a different type with constant struct list_head pointers, and
uses this pointer to call the functions, which trips indirect call
Control-Flow Integrity (CFI) checking.
Instead of removing the consts, this change defines the
list_cmp_func_t type and changes the comparison function types of
all list_sort() callers to use const pointers, thus avoiding type
mismatches.
Suggested-by: Nick Desaulniers
Signed-off-by: Sami Tolvanen
Reviewed-by: Nick Desaulniers
Tested-by: Nick Desaulniers
Reviewed-by: Christoph Hellwig
Reviewed-by: Kees Cook
---
arch/arm64/kvm/vgic/vgic-its.c | 8
arch/arm64/kvm/vgic/vgic.c | 3 ++-
block/blk-mq-sched.c| 3 ++-
block/blk-mq.c | 3 ++-
drivers/acpi/nfit/core.c| 3 ++-
drivers/acpi/numa/hmat.c| 3 ++-
drivers/clk/keystone/sci-clk.c | 4 ++--
drivers/gpu/drm/drm_modes.c | 3 ++-
drivers/gpu/drm/i915/gt/intel_engine_user.c | 3 ++-
drivers/gpu/drm/i915/gvt/debugfs.c | 2 +-
drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 3 ++-
drivers/gpu/drm/radeon/radeon_cs.c | 4 ++--
.../hw/usnic/usnic_uiom_interval_tree.c | 3 ++-
drivers/interconnect/qcom/bcm-voter.c | 2 +-
drivers/md/raid5.c | 3 ++-
drivers/misc/sram.c | 4 ++--
drivers/nvme/host/core.c| 3 ++-
.../pci/controller/cadence/pcie-cadence-host.c | 3 ++-
drivers/spi/spi-loopback-test.c | 3 ++-
fs/btrfs/raid56.c | 3 ++-
fs/btrfs/tree-log.c | 3 ++-
fs/btrfs/volumes.c | 3 ++-
fs/ext4/fsmap.c | 4 ++--
fs/gfs2/glock.c | 3 ++-
fs/gfs2/log.c | 2 +-
fs/gfs2/lops.c | 3 ++-
fs/iomap/buffered-io.c | 3 ++-
fs/ubifs/gc.c | 7 ---
fs/ubifs/replay.c | 4 ++--
fs/xfs/scrub/bitmap.c | 4 ++--
fs/xfs/xfs_bmap_item.c | 4 ++--
fs/xfs/xfs_buf.c| 6 +++---
fs/xfs/xfs_extent_busy.c| 4 ++--
fs/xfs/xfs_extent_busy.h| 3 ++-
fs/xfs/xfs_extfree_item.c | 4 ++--
fs/xfs/xfs_refcount_item.c | 4 ++--
fs/xfs/xfs_rmap_item.c | 4 ++--
include/linux/list_sort.h | 7 ---
lib/list_sort.c | 17 ++---
lib/test_list_sort.c| 3 ++-
net/tipc/name_table.c | 4 ++--
41 files changed, 90 insertions(+), 72 deletions(-)
diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index 40cbaca81333..b9518f94bd43 100644
--- a/arch/arm64/kvm/vgic/vgic-its.c
+++ b/arch/arm64/kvm/vgic/vgic-its.c
@@ -2190,8 +2190,8 @@ static int vgic_its_restore_ite(struct vgic_its *its, u32
event_id,
return offset;
}
-static int vgic_its_ite_cmp(void *priv, struct list_head *a,
- struct list_head *b)
+static int vgic_its_ite_cmp(void *priv, const struct list_head *a,
+ const struct list_head *b)
{
struct its_ite *itea = container_of(a, struct its_ite, ite_list);
struct its_ite *iteb = container_of(b, struct its_ite, ite_list);
@@ -2329,8 +2329,8 @@ static int vgic_its_restore_dte(struct vgic_its *its, u32
id,
return offset;
}
-static int vgic_its_device_cmp(void *priv, struct list_head *a,
- struct list_head *b)
+static int vgic_its_device_cmp(void *priv, const struct list_head *a,
+ const struct list_head *b)
{
struct its_device *deva = container_of(a, struct its_device, dev_list);
struct its_device *devb = container_of(b, struct its_device, dev_list);
diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
index 1c597c9885fa..15b666200f0b 100644
--- a/arch/arm64/kvm/vgic/vgic.c
+++ b/arch/arm64/kvm/vgic/vgic.c
@@ -255,7 +255,8 @@ static struct kvm_vcpu *vgic_target_oracle(struct vgic_irq
*irq)
* Return negative if "a" sorts before "b", 0 to preserve order, and positive
* to sort "b" before "a".
*/
-static int vgic_irq_cmp(void *priv, struct list_head *a, struct list_head *b)
+static int vgic_irq_cmp(void *priv, const struct list_head *a,
+ const struct list_head *b)
{
struct vgic_irq *irqa = container_of(a, struct vgic_irq, ap_list);
