From: Mark Rutland <mark.rutland@arm.com> Date: 2021-10-13 12:12:21
We recently realised that out-of-line extable fixups cause a number of problems
for backtracing (mattering both for developers and for RELIABLE_STACKTRACE and
LIVEPATCH). Dmitry spotted a confusing backtrace, which we identified was due
to problems with unwinding fixups, as summarized in:
https://lore.kernel.org/linux-arm-kernel/20210927171812.GB9201@C02TD0UTHF1T.local/
The gist is that while backtracing through a fixup, the fixup gets symmbolized
as an offset from the nearest prior symbol (which happens to be
`__entry_tramp_text_end`), and we the backtrace misses the function that was
being fixed up (because the fixup handling adjusts the PC, then the fixup does
a direct branch back to the original function). We can't reliably map from an
arbitrary PC in the fixup text back to the original function.
The way we create fixups is a bit unfortunate: most fixups are generated from
common templates, and only differ in register to be poked and the address to
branch back to, leading to redundant copies of the same logic that must pollute
Since the fixups are all written in assembly, and duplicated for each fixup
site, we can only perform very simple fixups, and can't handle any complex
triage that we might need for some exceptions (e.g. MTE faults).
This series address these concerns by getting rid of the out-of-line anonymous
fixup logic:
* For plain assembly functions, we move the fixup into the body of
the function, after the usual return, as we already do for our cache
routines. This simplifies the source code, and only adds a handful of
instructions to the main body of `.text`.
This is handled by the first three patches, which I think are trivial and
could be queued regardless of the rest of the series.
* For inline assembly, we add specialised handlers which run in exception
context to update registers, then adjust the PC *within* the faulting
function. This requires some new glue to capture the handler and metadata in
struct exception_table_entry (costing 32 bits per fixup), but for any
non-trivial fixup (which is all of the inline asm cases), this removes at
least two instructions of out-of-line fixup.
As the fixups are now handled from C code in exception context, we can more
easily extend these in future with more complex triage if necessary.
Overall, this doesn't have an appreciable impact on Image size (in local
testing the size of the Image was identical before/after), but does shift the
boundary between .text and .ordata, making .text smaller and .rodata bigger.
.text somewhat while growing .rodata somewhat.
I've tested this with both GCC and clang (including with clang CFI), and
everything is working as expected.
Other than changes to backtracing, there should be no functional change as a
result of this series.
Thanks
Mark.
Mark Rutland (13):
arm64: lib: __arch_clear_user(): fold fixups into body
arm64: lib: __arch_copy_from_user(): fold fixups into body
arm64: lib: __arch_copy_to_user(): fold fixups into body
arm64: kvm: use kvm_exception_table_entry
arm64: factor out GPR numbering helpers
arm64: gpr-num: support W registers
arm64: extable: consolidate definitions
arm64: extable: make fixup_exception() return bool
arm64: extable: use `ex` for `exception_table_entry`
arm64: extable: add `type` and `data` fields
arm64: extable: add a dedicated uaccess handler
arm64: extable: add load_unaligned_zeropad() handler
arm64: vmlinux.lds.S: remove `.fixup` section
arch/arm64/include/asm/asm-extable.h | 95 +++++++++++++++++++++++++++++++++
arch/arm64/include/asm/asm-uaccess.h | 7 ++-
arch/arm64/include/asm/assembler.h | 29 +---------
arch/arm64/include/asm/extable.h | 23 +++++---
arch/arm64/include/asm/futex.h | 25 +++------
arch/arm64/include/asm/gpr-num.h | 26 +++++++++
arch/arm64/include/asm/kvm_asm.h | 7 +--
arch/arm64/include/asm/sysreg.h | 25 +++------
arch/arm64/include/asm/uaccess.h | 26 ++-------
arch/arm64/include/asm/word-at-a-time.h | 21 ++------
arch/arm64/kernel/armv8_deprecated.c | 12 ++---
arch/arm64/kernel/traps.c | 9 +---
arch/arm64/kernel/vmlinux.lds.S | 1 -
arch/arm64/kvm/hyp/include/hyp/switch.h | 10 ++--
arch/arm64/lib/clear_user.S | 9 ++--
arch/arm64/lib/copy_from_user.S | 7 +--
arch/arm64/lib/copy_to_user.S | 7 +--
arch/arm64/mm/extable.c | 85 +++++++++++++++++++++++++----
arch/arm64/net/bpf_jit_comp.c | 9 ++--
scripts/sorttable.c | 30 +++++++++++
20 files changed, 306 insertions(+), 157 deletions(-)
create mode 100644 arch/arm64/include/asm/asm-extable.h
create mode 100644 arch/arm64/include/asm/gpr-num.h
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-10-13 12:14:06
Like other functions, __arch_copy_to_user() places its exception fixups
in the `.fixup` section without any clear association with
__arch_copy_to_user() itself. If we backtrace the fixup code, it will be
symbolized as an offset from the nearest prior symbol, which happens to
be `__entry_tramp_text_end`. Further, since the PC adjustment for the
fixup is akin to a direct branch rather than a function call,
__arch_copy_to_user() itself will be missing from the backtrace.
This is confusing and hinders debugging. In general this pattern will
also be problematic for CONFIG_LIVEPATCH, since fixups often return to
their associated function, but this isn't accurately captured in the
stacktrace.
To solve these issues for assembly functions, we must move fixups into
the body of the functions themselves, after the usual fast-path returns.
This patch does so for __arch_copy_to_user().
Inline assembly will be dealt with in subsequent patches.
Other than the improved backtracing, there should be no functional
change as a result of this patch.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
arch/arm64/lib/copy_to_user.S | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-10-13 12:15:52
Like other functions, __arch_clear_user() places its exception fixups in
the `.fixup` section without any clear association with
__arch_clear_user() itself. If we backtrace the fixup code, it will be
symbolized as an offset from the nearest prior symbol, which happens to
be `__entry_tramp_text_end`. Further, since the PC adjustment for the
fixup is akin to a direct branch rather than a function call,
__arch_clear_user() itself will be missing from the backtrace.
This is confusing and hinders debugging. In general this pattern will
also be problematic for CONFIG_LIVEPATCH, since fixups often return to
their associated function, but this isn't accurately captured in the
stacktrace.
To solve these issues for assembly functions, we must move fixups into
the body of the functions themselves, after the usual fast-path returns.
This patch does so for __arch_clear_user().
Inline assembly will be dealt with in subsequent patches.
Other than the improved backtracing, there should be no functional
change as a result of this patch.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
arch/arm64/lib/clear_user.S | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
@@ -45,13 +45,10 @@ USER(9f, sttrh wzr, [x0])USER(7f,sttrbwzr,[x2,#-1])5:movx0,#0ret-SYM_FUNC_END(__arch_clear_user)-EXPORT_SYMBOL(__arch_clear_user)-.section.fixup,"ax"-.align27:subx0,x2,#5 // Adjust for faulting on the final byte...8:addx0,x0,#4 // ...or the second word of the 4-7 byte case9:subx0,x2,x0ret-.previous+SYM_FUNC_END(__arch_clear_user)+EXPORT_SYMBOL(__arch_clear_user)
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-10-13 19:58:10
On 2021-10-13 12:00, Mark Rutland wrote:
Like other functions, __arch_clear_user() places its exception fixups in
the `.fixup` section without any clear association with
__arch_clear_user() itself. If we backtrace the fixup code, it will be
symbolized as an offset from the nearest prior symbol, which happens to
be `__entry_tramp_text_end`. Further, since the PC adjustment for the
fixup is akin to a direct branch rather than a function call,
__arch_clear_user() itself will be missing from the backtrace.
This is confusing and hinders debugging. In general this pattern will
also be problematic for CONFIG_LIVEPATCH, since fixups often return to
their associated function, but this isn't accurately captured in the
stacktrace.
To solve these issues for assembly functions, we must move fixups into
the body of the functions themselves, after the usual fast-path returns.
This patch does so for __arch_clear_user().
Inline assembly will be dealt with in subsequent patches.
Other than the improved backtracing, there should be no functional
change as a result of this patch.
Oh, I always assumed the .fixup section might have some special
significance of its own. If not, all the better - modulo one possible
nit below, this is fine by me. Also it's led me to see that
copy_in_user() has finally left us, hooray! Guess I've got no more
excuses to put off that promised usercopy rewrite other than finding the
time now...
quoted hunk
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
arch/arm64/lib/clear_user.S | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
The one useful purpose this did serve is to provide a handy visual cue -
if you have any more concrete reason to respin the series, would you
mind sticking in a little comment like "// Exception fixup" here and in
the other two assembly routines, just so it's harder to overlook that
the preceding ret is the normal exit path?
Either way, for patches 1-3,
Acked-by: Robin Murphy <robin.murphy@arm.com>
I got totally lost trying to follow the _ASM_EXTABLE_UACCESS_* business,
but I don't think the rest of the series gets in the way of any
outstanding plans either (and FWIW I always thought "fixup->fixup" was
pretty awful so feel free to have an ack for patch #9 as well). Indeed,
I guess the new type field should mean that we can implement "proper"
address-aware fault handlers without the Itanium trick if we still need to.
Cheers,
Robin
- .align 2
7: sub x0, x2, #5 // Adjust for faulting on the final byte...
8: add x0, x0, #4 // ...or the second word of the 4-7 byte case
9: sub x0, x2, x0
ret
- .previous
+SYM_FUNC_END(__arch_clear_user)
+EXPORT_SYMBOL(__arch_clear_user)
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-10-14 11:12:04
On Wed, Oct 13, 2021 at 08:55:31PM +0100, Robin Murphy wrote:
On 2021-10-13 12:00, Mark Rutland wrote:
quoted
Like other functions, __arch_clear_user() places its exception fixups in
the `.fixup` section without any clear association with
__arch_clear_user() itself. If we backtrace the fixup code, it will be
symbolized as an offset from the nearest prior symbol, which happens to
be `__entry_tramp_text_end`. Further, since the PC adjustment for the
fixup is akin to a direct branch rather than a function call,
__arch_clear_user() itself will be missing from the backtrace.
This is confusing and hinders debugging. In general this pattern will
also be problematic for CONFIG_LIVEPATCH, since fixups often return to
their associated function, but this isn't accurately captured in the
stacktrace.
To solve these issues for assembly functions, we must move fixups into
the body of the functions themselves, after the usual fast-path returns.
This patch does so for __arch_clear_user().
Inline assembly will be dealt with in subsequent patches.
Other than the improved backtracing, there should be no functional
change as a result of this patch.
Oh, I always assumed the .fixup section might have some special significance
of its own. If not, all the better - modulo one possible nit below, this is
fine by me.
Cheers, I'll go fix those up.
Also it's led me to see that copy_in_user() has finally left us,
hooray! Guess I've got no more excuses to put off that promised usercopy
rewrite other than finding the time now...
The one useful purpose this did serve is to provide a handy visual cue - if
you have any more concrete reason to respin the series, would you mind
sticking in a little comment like "// Exception fixup" here and in the other
two assembly routines, just so it's harder to overlook that the preceding
ret is the normal exit path?
I've added:
| // Exception fixups
... to the start of the fixups in patches 1-3.
Either way, for patches 1-3,
Acked-by: Robin Murphy <robin.murphy@arm.com>
Thanks!
I got totally lost trying to follow the _ASM_EXTABLE_UACCESS_* business, but
I don't think the rest of the series gets in the way of any outstanding
plans either (and FWIW I always thought "fixup->fixup" was pretty awful so
feel free to have an ack for patch #9 as well).
Thanks again!
Indeed, I guess the new type field should mean that we can implement
"proper" address-aware fault handlers without the Itanium trick if we
still need to.
Yes -- my thinking was that we can capture the address register(s) and
offset(s) in the data field, and *also* provide the ESR and/or FAR if
necessary in the exception handler.
What's the Itanium trick?
Thanks,
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-10-13 12:18:01
Like other functions, __arch_copy_from_user() places its exception
fixups in the `.fixup` section without any clear association with
__arch_copy_from_user() itself. If we backtrace the fixup code, it will
be symbolized as an offset from the nearest prior symbol, which happens
to be `__entry_tramp_text_end`. Further, since the PC adjustment for the
fixup is akin to a direct branch rather than a function call,
__arch_copy_from_user() itself will be missing from the backtrace.
This is confusing and hinders debugging. In general this pattern will
also be problematic for CONFIG_LIVEPATCH, since fixups often return to
their associated function, but this isn't accurately captured in the
stacktrace.
To solve these issues for assembly functions, we must move fixups into
the body of the functions themselves, after the usual fast-path returns.
This patch does so for __arch_copy_from_user().
Inline assembly will be dealt with in subsequent patches.
Other than the improved backtracing, there should be no functional
change as a result of this patch.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
arch/arm64/lib/copy_from_user.S | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-10-13 12:19:41
In subsequent patches we'll alter `struct exception_table_entry`, adding
fields that are not needed for KVM exception fixups.
In preparation for this, migrate KVM to its own `struct
kvm_exception_table_entry`, which is identical to the current format of
`struct exception_table_entry`. Comments are updated accordingly.
There should be no functional change as a result of this patch.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Alexandru Elisei <redacted>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/kvm_asm.h | 7 ++++---
arch/arm64/kvm/hyp/include/hyp/switch.h | 10 +++++++---
2 files changed, 11 insertions(+), 6 deletions(-)
@@ -30,8 +30,12 @@#include<asm/processor.h>#include<asm/thread_info.h>-externstructexception_table_entry__start___kvm_ex_table;-externstructexception_table_entry__stop___kvm_ex_table;+structkvm_exception_table_entry{+intinsn,fixup;+};++externstructkvm_exception_table_entry__start___kvm_ex_table;+externstructkvm_exception_table_entry__stop___kvm_ex_table;/* Check whether the FP regs were dirtied while in the host-side run loop: */staticinlineboolupdate_fp_enabled(structkvm_vcpu*vcpu)
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-10-13 12:21:29
In <asm/sysreg.h> we have macros to convert the names of general purpose
registers (GPRs) into integer constants, which we use to manually build
the encoding for `MRS` and `MSR` instructions where we can't rely on the
assembler to do so for us.
In subsequent patches we'll need to map the same GPR names to integer
constants so that we can use this to build metadata for exception
fixups.
So that the we can use the mappings elsewhere, factor out the
definitions into a new <asm/gpr-num.h> header, renaming the definitions
to align with this "GPR num" naming for clarity.
There should be no functional change as a result of this patch.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/gpr-num.h | 22 ++++++++++++++++++++++
arch/arm64/include/asm/sysreg.h | 25 ++++++++-----------------
2 files changed, 30 insertions(+), 17 deletions(-)
create mode 100644 arch/arm64/include/asm/gpr-num.h
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-10-13 12:24:46
In subsequent patches we'll want to map W registers to their register
numbers. Update gpr-num.h so that we can do this.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/gpr-num.h | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-10-13 12:25:54
In subsequent patches we'll alter the structure and usage of struct
exception_table_entry. For inline assembly, we create these using the
`_ASM_EXTABLE()` CPP macro defined in <asm/uaccess.h>, and for plain
assembly code we use the `_asm_extable()` GAS macro defined in
<asm/assembler.h>, which are largely identical save for different
escaping and stringification requirements.
This patch moves the common definitions to a new <asm/asm-extable.h>
header, so that it's easier to keep the two in-sync, and to remove the
implication that these are only used for uaccess helpers (as e.g.
load_unaligned_zeropad() is only used on kernel memory, and depends upon
`_ASM_EXTABLE()`.
At the same time, a few minor modifications are made for clarity and in
preparation for subsequent patches:
* The structure creation is factored out into an `__ASM_EXTABLE_RAW()`
macro. This will make it easier to support different fixup variants in
subsequent patches without needing to update all users of
`_ASM_EXTABLE()`, and makes it easier to see tha the CPP and GAS
variants of the macros are structurally identical.
For the CPP macro, the stringification of fields is left to the
wrapper macro, `_ASM_EXTABLE()`, as in subsequent patches it will be
necessary to stringify fields in wrapper macros to safely concatenate
strings which cannot be token-pasted together in CPP.
* The fields of the structure are created separately on their own lines.
This will make it easier to add/remove/modify individual fields
clearly.
* Additional parentheses are added around the use of macro arguments in
field definitions to avoid any potential problems with evaluation due
to operator precedence, and to make errors upon misuse clearer.
* USER() is moved into <asm/asm-uaccess.h>, as it is not required by all
assembly code, and is already refered to by comments in that file.
There should be no functional change as a result of this patch.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/asm-extable.h | 48 ++++++++++++++++++++++++++++++++++++
arch/arm64/include/asm/asm-uaccess.h | 7 +++++-
arch/arm64/include/asm/assembler.h | 29 ++--------------------
arch/arm64/include/asm/uaccess.h | 7 +-----
arch/arm64/lib/clear_user.S | 2 +-
5 files changed, 58 insertions(+), 35 deletions(-)
create mode 100644 arch/arm64/include/asm/asm-extable.h
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-10-13 12:28:05
The return values of fixup_exception() and arm64_bpf_fixup_exception()
represent a boolean condition rather than an error code, so for clarity
it would be better to return `bool` rather than `int`.
This patch adjusts the code accordingly. While we're modifying the
prototype, we also remove the unnecessary `extern` keyword, so that this
won't look out of place when we make subsequent additions to the header.
There should be no functional change as a result of this patch.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: James Morse <james.morse@arm.com>
Cc: Jean-Philippe Brucker <redacted>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/extable.h | 10 +++++-----
arch/arm64/mm/extable.c | 6 +++---
arch/arm64/net/bpf_jit_comp.c | 6 +++---
3 files changed, 11 insertions(+), 11 deletions(-)
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-10-13 12:30:09
Subsequent patches will extend `struct exception_table_entry` with more
fields, and the distinction between the entry and its `fixup` field will
become more important.
For clarity, let's consistently use `ex` to refer to refer to an entire
entry. In subsequent patches we'll use `fixup` to refer to the fixup
field specifically. This matches the naming convention used today in
arch/arm64/net/bpf_jit_comp.c.
There should be no functional change as a result of this patch.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
arch/arm64/mm/extable.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-10-13 12:30:58
Subsequent patches will add specialized handlers for fixups, in addition
to the simple PC fixup and BPF handlers we have today. In preparation,
this patch adds a new `type` field to struct exception_table_entry, and
uses this to distinguish the fixup and BPF cases. A `data` field is also
added so that subsequent patches can associate data specific to each
exception site (e.g. register numbers).
Handlers are named ex_handler_*() for consistency, following the exmaple
of x86. At the same time, get_ex_fixup() is split out into a helper so
that it can be used by other ex_handler_*() functions ins subsequent
patches.
This patch will increase the size of the exception tables, which will be
remedied by subsequent patches removing redundant fixup code. There
should be no functional change as a result of this patch.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: James Morse <james.morse@arm.com>
Cc: Jean-Philippe Brucker <redacted>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/asm-extable.h | 32 ++++++++++++++++++++------------
arch/arm64/include/asm/extable.h | 19 +++++++++++++++----
arch/arm64/mm/extable.c | 29 +++++++++++++++++++++++++----
arch/arm64/net/bpf_jit_comp.c | 7 +++++--
scripts/sorttable.c | 30 ++++++++++++++++++++++++++++++
5 files changed, 95 insertions(+), 22 deletions(-)
@@ -231,6 +231,34 @@ static void sort_relative_table(char *extab_image, int image_size)}}+staticvoidarm64_sort_relative_table(char*extab_image,intimage_size)+{+inti=0;++while(i<image_size){+uint32_t*loc=(uint32_t*)(extab_image+i);++w(r(loc)+i,loc);+w(r(loc+1)+i+4,loc+1);+/* Don't touch the fixup type or data */++i+=sizeof(uint32_t)*3;+}++qsort(extab_image,image_size/12,12,compare_relative_table);++i=0;+while(i<image_size){+uint32_t*loc=(uint32_t*)(extab_image+i);++w(r(loc)-i,loc);+w(r(loc+1)-(i+4),loc+1);+/* Don't touch the fixup type or data */++i+=sizeof(uint32_t)*3;+}+}+staticvoidx86_sort_relative_table(char*extab_image,intimage_size){inti=0;
From: Will Deacon <will@kernel.org> Date: 2021-10-19 11:31:13
On Wed, Oct 13, 2021 at 12:00:56PM +0100, Mark Rutland wrote:
quoted hunk
Subsequent patches will add specialized handlers for fixups, in addition
to the simple PC fixup and BPF handlers we have today. In preparation,
this patch adds a new `type` field to struct exception_table_entry, and
uses this to distinguish the fixup and BPF cases. A `data` field is also
added so that subsequent patches can associate data specific to each
exception site (e.g. register numbers).
Handlers are named ex_handler_*() for consistency, following the exmaple
of x86. At the same time, get_ex_fixup() is split out into a helper so
that it can be used by other ex_handler_*() functions ins subsequent
patches.
This patch will increase the size of the exception tables, which will be
remedied by subsequent patches removing redundant fixup code. There
should be no functional change as a result of this patch.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: James Morse <james.morse@arm.com>
Cc: Jean-Philippe Brucker <redacted>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/asm-extable.h | 32 ++++++++++++++++++++------------
arch/arm64/include/asm/extable.h | 19 +++++++++++++++----
arch/arm64/mm/extable.c | 29 +++++++++++++++++++++++++----
arch/arm64/net/bpf_jit_comp.c | 7 +++++--
scripts/sorttable.c | 30 ++++++++++++++++++++++++++++++
5 files changed, 95 insertions(+), 22 deletions(-)
@@ -231,6 +231,34 @@ static void sort_relative_table(char *extab_image, int image_size)}}+staticvoidarm64_sort_relative_table(char*extab_image,intimage_size)+{+inti=0;++while(i<image_size){+uint32_t*loc=(uint32_t*)(extab_image+i);++w(r(loc)+i,loc);+w(r(loc+1)+i+4,loc+1);+/* Don't touch the fixup type or data */++i+=sizeof(uint32_t)*3;+}++qsort(extab_image,image_size/12,12,compare_relative_table);++i=0;+while(i<image_size){+uint32_t*loc=(uint32_t*)(extab_image+i);++w(r(loc)-i,loc);+w(r(loc+1)-(i+4),loc+1);+/* Don't touch the fixup type or data */++i+=sizeof(uint32_t)*3;+}+}
This is very nearly a direct copy of x86_sort_relative_table() (magic
numbers and all). It would be nice to tidy that up, but I couldn't
immediately see a good way to do it :(
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-10-19 11:51:51
On Tue, Oct 19, 2021 at 12:29:55PM +0100, Will Deacon wrote:
On Wed, Oct 13, 2021 at 12:00:56PM +0100, Mark Rutland wrote:
quoted
Subsequent patches will add specialized handlers for fixups, in addition
to the simple PC fixup and BPF handlers we have today. In preparation,
this patch adds a new `type` field to struct exception_table_entry, and
uses this to distinguish the fixup and BPF cases. A `data` field is also
added so that subsequent patches can associate data specific to each
exception site (e.g. register numbers).
Handlers are named ex_handler_*() for consistency, following the exmaple
of x86. At the same time, get_ex_fixup() is split out into a helper so
that it can be used by other ex_handler_*() functions ins subsequent
patches.
This patch will increase the size of the exception tables, which will be
remedied by subsequent patches removing redundant fixup code. There
should be no functional change as a result of this patch.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: James Morse <james.morse@arm.com>
Cc: Jean-Philippe Brucker <redacted>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/asm-extable.h | 32 ++++++++++++++++++++------------
arch/arm64/include/asm/extable.h | 19 +++++++++++++++----
arch/arm64/mm/extable.c | 29 +++++++++++++++++++++++++----
arch/arm64/net/bpf_jit_comp.c | 7 +++++--
scripts/sorttable.c | 30 ++++++++++++++++++++++++++++++
5 files changed, 95 insertions(+), 22 deletions(-)
That's because the size of each entry is now 12 bytes, and
`.align 3` aligns to 8 bytes, which would leave a gap between entries.
We only require the fields are naturally aligned, so `.align 2` is
sufficient, and doesn't waste space.
I'll update the commit message to call that out.
@@ -231,6 +231,34 @@ static void sort_relative_table(char *extab_image, int image_size)}}+staticvoidarm64_sort_relative_table(char*extab_image,intimage_size)+{+inti=0;++while(i<image_size){+uint32_t*loc=(uint32_t*)(extab_image+i);++w(r(loc)+i,loc);+w(r(loc+1)+i+4,loc+1);+/* Don't touch the fixup type or data */++i+=sizeof(uint32_t)*3;+}++qsort(extab_image,image_size/12,12,compare_relative_table);++i=0;+while(i<image_size){+uint32_t*loc=(uint32_t*)(extab_image+i);++w(r(loc)-i,loc);+w(r(loc+1)-(i+4),loc+1);+/* Don't touch the fixup type or data */++i+=sizeof(uint32_t)*3;+}+}
This is very nearly a direct copy of x86_sort_relative_table() (magic
numbers and all). It would be nice to tidy that up, but I couldn't
immediately see a good way to do it :(
Beware that's true in linux-next, but not mainline, as that changes in
commit:
46d28947d9876fc0 ("x86/extable: Rework the exception table mechanics")
A patch to unify the two is trivial, but will cause a cross-tree
dependency, so I'd suggest having this separate for now and sending a
unification patch come -rc1.
I can note something to that effect in the commit message, if that
helps?
Thanks,
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-10-19 12:06:30
On Tue, Oct 19, 2021 at 12:50:22PM +0100, Mark Rutland wrote:
On Tue, Oct 19, 2021 at 12:29:55PM +0100, Will Deacon wrote:
quoted
On Wed, Oct 13, 2021 at 12:00:56PM +0100, Mark Rutland wrote:
quoted
Subsequent patches will add specialized handlers for fixups, in addition
to the simple PC fixup and BPF handlers we have today. In preparation,
this patch adds a new `type` field to struct exception_table_entry, and
uses this to distinguish the fixup and BPF cases. A `data` field is also
added so that subsequent patches can associate data specific to each
exception site (e.g. register numbers).
Handlers are named ex_handler_*() for consistency, following the exmaple
of x86. At the same time, get_ex_fixup() is split out into a helper so
that it can be used by other ex_handler_*() functions ins subsequent
patches.
This patch will increase the size of the exception tables, which will be
remedied by subsequent patches removing redundant fixup code. There
should be no functional change as a result of this patch.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: James Morse <james.morse@arm.com>
Cc: Jean-Philippe Brucker <redacted>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/asm-extable.h | 32 ++++++++++++++++++++------------
arch/arm64/include/asm/extable.h | 19 +++++++++++++++----
arch/arm64/mm/extable.c | 29 +++++++++++++++++++++++++----
arch/arm64/net/bpf_jit_comp.c | 7 +++++--
scripts/sorttable.c | 30 ++++++++++++++++++++++++++++++
5 files changed, 95 insertions(+), 22 deletions(-)
That's because the size of each entry is now 12 bytes, and
`.align 3` aligns to 8 bytes, which would leave a gap between entries.
We only require the fields are naturally aligned, so `.align 2` is
sufficient, and doesn't waste space.
I'll update the commit message to call that out.
I think the part which is confusing me is that I would expect the alignment
here to match the alignment of the corresponding C type, but the old value
of '3' doesn't seem to do that, so is this patch fixing an earlier bug?
Without your patches in the picture, we're using a '.align 3' in
_asm_extable, but with:
struct exception_table_entry
{
int insn, fixup;
};
I suppose it works out because that over-alignment doesn't result in any
additional padding, but I think we could reduce the current alignment
without any of these other changes, no?
@@ -231,6 +231,34 @@ static void sort_relative_table(char *extab_image, int image_size)}}+staticvoidarm64_sort_relative_table(char*extab_image,intimage_size)+{+inti=0;++while(i<image_size){+uint32_t*loc=(uint32_t*)(extab_image+i);++w(r(loc)+i,loc);+w(r(loc+1)+i+4,loc+1);+/* Don't touch the fixup type or data */++i+=sizeof(uint32_t)*3;+}++qsort(extab_image,image_size/12,12,compare_relative_table);++i=0;+while(i<image_size){+uint32_t*loc=(uint32_t*)(extab_image+i);++w(r(loc)-i,loc);+w(r(loc+1)-(i+4),loc+1);+/* Don't touch the fixup type or data */++i+=sizeof(uint32_t)*3;+}+}
This is very nearly a direct copy of x86_sort_relative_table() (magic
numbers and all). It would be nice to tidy that up, but I couldn't
immediately see a good way to do it :(
Beware that's true in linux-next, but not mainline, as that changes in
commit:
46d28947d9876fc0 ("x86/extable: Rework the exception table mechanics")
A patch to unify the two is trivial, but will cause a cross-tree
dependency, so I'd suggest having this separate for now and sending a
unification patch come -rc1.
I can note something to that effect in the commit message, if that
helps?
Yeah, I suppose. It's not worth tripping over the x86 changes, but we
should try to remember to come back and unify things.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, 19 Oct 2021 at 14:05, Will Deacon [off-list ref] wrote:
On Tue, Oct 19, 2021 at 12:50:22PM +0100, Mark Rutland wrote:
quoted
On Tue, Oct 19, 2021 at 12:29:55PM +0100, Will Deacon wrote:
quoted
On Wed, Oct 13, 2021 at 12:00:56PM +0100, Mark Rutland wrote:
quoted
Subsequent patches will add specialized handlers for fixups, in addition
to the simple PC fixup and BPF handlers we have today. In preparation,
this patch adds a new `type` field to struct exception_table_entry, and
uses this to distinguish the fixup and BPF cases. A `data` field is also
added so that subsequent patches can associate data specific to each
exception site (e.g. register numbers).
Handlers are named ex_handler_*() for consistency, following the exmaple
of x86. At the same time, get_ex_fixup() is split out into a helper so
that it can be used by other ex_handler_*() functions ins subsequent
patches.
This patch will increase the size of the exception tables, which will be
remedied by subsequent patches removing redundant fixup code. There
should be no functional change as a result of this patch.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: James Morse <james.morse@arm.com>
Cc: Jean-Philippe Brucker <redacted>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/asm-extable.h | 32 ++++++++++++++++++++------------
arch/arm64/include/asm/extable.h | 19 +++++++++++++++----
arch/arm64/mm/extable.c | 29 +++++++++++++++++++++++++----
arch/arm64/net/bpf_jit_comp.c | 7 +++++--
scripts/sorttable.c | 30 ++++++++++++++++++++++++++++++
5 files changed, 95 insertions(+), 22 deletions(-)
That's because the size of each entry is now 12 bytes, and
`.align 3` aligns to 8 bytes, which would leave a gap between entries.
We only require the fields are naturally aligned, so `.align 2` is
sufficient, and doesn't waste space.
I'll update the commit message to call that out.
I think the part which is confusing me is that I would expect the alignment
here to match the alignment of the corresponding C type, but the old value
of '3' doesn't seem to do that, so is this patch fixing an earlier bug?
Without your patches in the picture, we're using a '.align 3' in
_asm_extable, but with:
struct exception_table_entry
{
int insn, fixup;
};
I suppose it works out because that over-alignment doesn't result in any
additional padding, but I think we could reduce the current alignment
without any of these other changes, no?
If a struct type's size happens to be a power of 2, it is perfectly
reasonable to align it to its size rather than use the minimum
alignment required by its members, as this will generally result in
more efficient placement wrt cachelines, pages, etc.
So the change is obviously needed here, but I wouldn't consider the
old alignment value to be a bug.
--
Ard.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
That's because the size of each entry is now 12 bytes, and
`.align 3` aligns to 8 bytes, which would leave a gap between entries.
We only require the fields are naturally aligned, so `.align 2` is
sufficient, and doesn't waste space.
I'll update the commit message to call that out.
I think the part which is confusing me is that I would expect the alignment
here to match the alignment of the corresponding C type, but the old value
of '3' doesn't seem to do that, so is this patch fixing an earlier bug?
Without your patches in the picture, we're using a '.align 3' in
_asm_extable, but with:
struct exception_table_entry
{
int insn, fixup;
};
I suppose it works out because that over-alignment doesn't result in any
additional padding, but I think we could reduce the current alignment
without any of these other changes, no?
Yes, we could reduce that first, but no, it's not a bug -- there's no
functional issue today.
For context, today the `__ex_table` section as a whole and the
`__start___ex_table` symbol also got 8 byte alignment, since in
ARch/arm64/kernel/vmlinux.lds.S we have:
| #define RO_EXCEPTION_TABLE_ALIGN 8
... and so in include/asm-generic/vmlinux.lds.h when the exception table
gets output with:
| EXCEPTION_TABLE(RO_EXCEPTION_TABLE_ALIGN)
| #define EXCEPTION_TABLE(align) \
| . = ALIGN(align); \
| __ex_table : AT(ADDR(__ex_table) - LOAD_OFFSET) { \
| __start___ex_table = .; \
| KEEP(*(__ex_table)) \
| __stop___ex_table = .; \
| }
If you want, I can split out a preparatory patch which drops the
alignment to the minimum necessary, both in the asm and for
RO_EXCEPTION_TABLE_ALIGN?
[...]
quoted
quoted
quoted
+static void arm64_sort_relative_table(char *extab_image, int image_size)
+{
+ int i = 0;
+
+ while (i < image_size) {
+ uint32_t *loc = (uint32_t *)(extab_image + i);
+
+ w(r(loc) + i, loc);
+ w(r(loc + 1) + i + 4, loc + 1);
+ /* Don't touch the fixup type or data */
+
+ i += sizeof(uint32_t) * 3;
+ }
+
+ qsort(extab_image, image_size / 12, 12, compare_relative_table);
+
+ i = 0;
+ while (i < image_size) {
+ uint32_t *loc = (uint32_t *)(extab_image + i);
+
+ w(r(loc) - i, loc);
+ w(r(loc + 1) - (i + 4), loc + 1);
+ /* Don't touch the fixup type or data */
+
+ i += sizeof(uint32_t) * 3;
+ }
+}
This is very nearly a direct copy of x86_sort_relative_table() (magic
numbers and all). It would be nice to tidy that up, but I couldn't
immediately see a good way to do it :(
Beware that's true in linux-next, but not mainline, as that changes in
commit:
46d28947d9876fc0 ("x86/extable: Rework the exception table mechanics")
A patch to unify the two is trivial, but will cause a cross-tree
dependency, so I'd suggest having this separate for now and sending a
unification patch come -rc1.
I can note something to that effect in the commit message, if that
helps?
Yeah, I suppose. It's not worth tripping over the x86 changes, but we
should try to remember to come back and unify things.
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-10-13 12:32:27
For inline assembly, we place exception fixups out-of-line in the
`.fixup` section such that these are out of the way of the fast path.
This has a few drawbacks:
* Since the fixup code is anonymous, backtraces will symbolize fixups as
offsets from the nearest prior symbol, currently
`__entry_tramp_text_end`. This is confusing, and painful to debug
without access to the relevant vmlinux.
* Since the exception handler adjusts the PC to execute the fixup, and
the fixup uses a direct branch back into the function it fixes,
backtraces of fixups miss the original function. This is confusing,
and violates requirements for RELIABLE_STACKTRACE (and therefore
LIVEPATCH).
* Inline assembly and associated fixups are generated from templates,
and we have many copies of logically identical fixups which only
differ in which specific registers are written to and which address is
branched to at the end of the fixup. This is potentially wasteful of
I-cache resources, and makes it hard to add additional logic to fixups
without significant bloat.
This patch address all three concerns for inline uaccess fixups by
adding a dedicated exception handler which updates registers in
exception context and subsequent returns back into the function which
faulted, removing the need for fixups specialized to each faulting
instruction.
Other than backtracing, there should be no functional change as a result
of this patch.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/asm-extable.h | 24 ++++++++++++++++++++++++
arch/arm64/include/asm/futex.h | 25 ++++++++-----------------
arch/arm64/include/asm/uaccess.h | 19 ++++---------------
arch/arm64/kernel/armv8_deprecated.c | 12 +++---------
arch/arm64/kernel/traps.c | 9 ++-------
arch/arm64/mm/extable.c | 17 +++++++++++++++++
6 files changed, 58 insertions(+), 48 deletions(-)
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-10-13 12:33:21
For inline assembly, we place exception fixups out-of-line in the
`.fixup` section such that these are out of the way of the fast path.
This has a few drawbacks:
* Since the fixup code is anonymous, backtraces will symbolize fixups as
offsets from the nearest prior symbol, currently
`__entry_tramp_text_end`. This is confusing, and painful to debug
without access to the relevant vmlinux.
* Since the exception handler adjusts the PC to execute the fixup, and
the fixup uses a direct branch back into the function it fixes,
backtraces of fixups miss the original function. This is confusing,
and violates requirements for RELIABLE_STACKTRACE (and therefore
LIVEPATCH).
* Inline assembly and associated fixups are generated from templates,
and we have many copies of logically identical fixups which only
differ in which specific registers are written to and which address is
branched to at the end of the fixup. This is potentially wasteful of
I-cache resources, and makes it hard to add additional logic to fixups
without significant bloat.
* In the case of load_unaligned_zeropad(), the logic in the fixup
requires a temporary register that we must allocate even in the
fast-path where it will not be used.
This patch address all four concerns for load_unaligned_zeropad() fixups
by adding a dedicated exception handler which performs the fixup logic
in exception context and subsequent returns back after the faulting
instruction. For the moment, the fixup logic is identical to the old
assembly fixup logic, but in future we could enhance this by taking the
ESR and FAR into account to constrain the faults we try to fix up, or to
specialize fixups for MTE tag check faults.
Other than backtracing, there should be no functional change as a result
of this patch.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/asm-extable.h | 15 +++++++++++++++
arch/arm64/include/asm/word-at-a-time.h | 21 ++++-----------------
arch/arm64/mm/extable.c | 29 +++++++++++++++++++++++++++++
3 files changed, 48 insertions(+), 17 deletions(-)
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-10-13 12:34:41
We no longer place anything into a `.fixup` section, so we no longer
need to place those sections into the `.text` section in the main kernel
Image.
Remove the use of `.fixup`.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
arch/arm64/kernel/vmlinux.lds.S | 1 -
1 file changed, 1 deletion(-)
On Wed, 13 Oct 2021 at 13:01, Mark Rutland [off-list ref] wrote:
We recently realised that out-of-line extable fixups cause a number of problems
for backtracing (mattering both for developers and for RELIABLE_STACKTRACE and
LIVEPATCH). Dmitry spotted a confusing backtrace, which we identified was due
to problems with unwinding fixups, as summarized in:
https://lore.kernel.org/linux-arm-kernel/20210927171812.GB9201@C02TD0UTHF1T.local/
The gist is that while backtracing through a fixup, the fixup gets symmbolized
as an offset from the nearest prior symbol (which happens to be
`__entry_tramp_text_end`), and we the backtrace misses the function that was
being fixed up (because the fixup handling adjusts the PC, then the fixup does
a direct branch back to the original function). We can't reliably map from an
arbitrary PC in the fixup text back to the original function.
The way we create fixups is a bit unfortunate: most fixups are generated from
common templates, and only differ in register to be poked and the address to
branch back to, leading to redundant copies of the same logic that must pollute
Since the fixups are all written in assembly, and duplicated for each fixup
site, we can only perform very simple fixups, and can't handle any complex
triage that we might need for some exceptions (e.g. MTE faults).
This series address these concerns by getting rid of the out-of-line anonymous
fixup logic:
* For plain assembly functions, we move the fixup into the body of
the function, after the usual return, as we already do for our cache
routines. This simplifies the source code, and only adds a handful of
instructions to the main body of `.text`.
This is handled by the first three patches, which I think are trivial and
could be queued regardless of the rest of the series.
* For inline assembly, we add specialised handlers which run in exception
context to update registers, then adjust the PC *within* the faulting
function. This requires some new glue to capture the handler and metadata in
struct exception_table_entry (costing 32 bits per fixup), but for any
non-trivial fixup (which is all of the inline asm cases), this removes at
least two instructions of out-of-line fixup.
As the fixups are now handled from C code in exception context, we can more
easily extend these in future with more complex triage if necessary.
Overall, this doesn't have an appreciable impact on Image size (in local
testing the size of the Image was identical before/after), but does shift the
boundary between .text and .ordata, making .text smaller and .rodata bigger.
.text somewhat while growing .rodata somewhat.
I've tested this with both GCC and clang (including with clang CFI), and
everything is working as expected.
Other than changes to backtracing, there should be no functional change as a
result of this series.
Thanks
Mark.
Mark Rutland (13):
arm64: lib: __arch_clear_user(): fold fixups into body
arm64: lib: __arch_copy_from_user(): fold fixups into body
arm64: lib: __arch_copy_to_user(): fold fixups into body
arm64: kvm: use kvm_exception_table_entry
arm64: factor out GPR numbering helpers
arm64: gpr-num: support W registers
arm64: extable: consolidate definitions
arm64: extable: make fixup_exception() return bool
arm64: extable: use `ex` for `exception_table_entry`
arm64: extable: add `type` and `data` fields
arm64: extable: add a dedicated uaccess handler
arm64: extable: add load_unaligned_zeropad() handler
arm64: vmlinux.lds.S: remove `.fixup` section
This looks like a very worthwhile improvement to me, as it moves
complexity from the asm fault sites to the C handling code.
For the series,
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>