2023-10-11 10:09:48

by Jinjie Ruan

[permalink] [raw]
Subject: [PATCH v5.15 00/15] arm64: Fix a concurrency issue in emulation_proc_handler()

MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In linux-6.1, the related code is refactored in commit 124c49b1b5d9
("arm64: armv8_deprecated: rework deprected instruction handling") and this
issue was incidentally fixed. This patch set try to adapt the refactoring
patches to stable 5.15 to solve the problem of repeated addition of linked
lists described below.

How to reproduce:
CONFIG_ARMV8_DEPRECATED=y, CONFIG_SWP_EMULATION=y, and CONFIG_DEBUG_LIST=y,
then launch two shell executions:
#!/bin/bash
while [ 1 ];
do
echo 1 > /proc/sys/abi/swp
done

or "echo 1 > /proc/sys/abi/swp" and then launch two shell executions:
#!/bin/bash
while [ 1 ];
do
echo 0 > /proc/sys/abi/swp
done

In emulation_proc_handler(), read and write operations are performed on
insn->current_mode. In the concurrency scenario, mutex only protects
writing insn->current_mode, and not protects the read. Suppose there are
two concurrent tasks, task1 updates insn->current_mode to INSN_EMULATE
in the critical section, the prev_mode of task2 is still the old data
INSN_UNDEF of insn->current_mode. As a result, two tasks call
update_insn_emulation_mode twice with prev_mode = INSN_UNDEF and
current_mode = INSN_EMULATE, then call register_emulation_hooks twice,
resulting in a list_add double problem.

commit 124c49b1b5d9 ("arm64: armv8_deprecated: rework deprected instruction
handling") remove the dynamic registration and unregistration so remove the
register_undef_hook() function, so the below problem was incidentally
fixed.

Call trace:
__list_add_valid+0xd8/0xe4
register_undef_hook+0x94/0x13c
update_insn_emulation_mode+0xd0/0x12c
emulation_proc_handler+0xd8/0xf4
proc_sys_call_handler+0x140/0x250
proc_sys_write+0x1c/0x2c
new_sync_write+0xec/0x18c
vfs_write+0x214/0x2ac
ksys_write+0x70/0xfc
__arm64_sys_write+0x24/0x30
el0_svc_common.constprop.0+0x7c/0x1bc
do_el0_svc+0x2c/0x94
el0_svc+0x20/0x30
el0_sync_handler+0xb0/0xb4
el0_sync+0x160/0x180

Call trace:
__list_del_entry_valid+0xac/0x110
unregister_undef_hook+0x34/0x80
update_insn_emulation_mode+0xf0/0x180
emulation_proc_handler+0x8c/0xd8
proc_sys_call_handler+0x1d8/0x208
proc_sys_write+0x14/0x20
new_sync_write+0xf0/0x190
vfs_write+0x304/0x388
ksys_write+0x6c/0x100
__arm64_sys_write+0x1c/0x28
el0_svc_common.constprop.4+0x68/0x188
do_el0_svc+0x24/0xa0
el0_svc+0x14/0x20
el0_sync_handler+0x90/0xb8
el0_sync+0x160/0x180

The first 5 patches is a patch set which provides context for subsequent
refactoring 9 patches, especially commit 0f2cb928a154 ("arm64:
consistently pass ESR_ELx to die()") which modify do_undefinstr() to add a
ESR_ELx value arg, and then commit 61d64a376ea8 ("arm64: split EL0/EL1
UNDEF handlers") splits do_undefinstr() handler into separate
do_el0_undef() and do_el1_undef() handlers.

The 9 patches after that is another refactoring patch set, which is in
preparation for the main rework commit 124c49b1b5d9 ("arm64:
armv8_deprecated: rework deprected instruction handling"). To remove struct
undef_hook, commit bff8f413c71f ("arm64: factor out EL1 SSBS emulation
hook") factor out EL1 SSBS emulation hook, which also avoid call
call_undef_hook() in do_el1_undef(), commit f5962add74b6 ("arm64: rework
EL0 MRS emulation") factor out EL0 MRS emulation hook, which also prepare
for replacing call_undef_hook() in do_el0_undef(). To replace
call_undef_hook() function, commit 61d64a376ea8 ("arm64: split EL0/EL1
UNDEF handlers") split the do_undefinstr() into do_el0_undef() and
do_el1_undef() functions, and commit dbfbd87efa79 ("arm64: factor insn
read out of call_undef_hook()") factor user_insn_read() from
call_undef_hook() so the main rework patch can replace the
call_undef_hook() in do_el0_undef().

The last patch is a bugfix for the main rework patch.

I've tested this with userspace programs which use each of the
deprecated instructions on Raspberry Pi 4B KVM/Qemu, and I've concurrently
modified the support level for each of the features back-and-forth between
HW and emulated to check that there are no oops or above repeated addition
or deletion call trace.

Fixes: af483947d472 ("arm64: fix oops in concurrently setting insn_emulation sysctls")
Cc: [email protected]#5.15.x
Cc: [email protected]
Signed-off-by: Jinjie Ruan <[email protected]>

Mark Rutland (14):
arm64: report EL1 UNDEFs better
arm64: die(): pass 'err' as long
arm64: consistently pass ESR_ELx to die()
arm64: rework FPAC exception handling
arm64: rework BTI exception handling
arm64: allow kprobes on EL0 handlers
arm64: split EL0/EL1 UNDEF handlers
arm64: factor out EL1 SSBS emulation hook
arm64: factor insn read out of call_undef_hook()
arm64: rework EL0 MRS emulation
arm64: armv8_deprecated: fold ops into insn_emulation
arm64: armv8_deprecated move emulation functions
arm64: armv8_deprecated: move aarch32 helper earlier
arm64: armv8_deprecated: rework deprected instruction handling

Ren Zhijie (1):
arm64: armv8_deprecated: fix unused-function error

arch/arm64/include/asm/cpufeature.h | 3 +-
arch/arm64/include/asm/exception.h | 13 +-
arch/arm64/include/asm/spectre.h | 2 +
arch/arm64/include/asm/system_misc.h | 2 +-
arch/arm64/include/asm/traps.h | 19 +-
arch/arm64/kernel/armv8_deprecated.c | 572 +++++++++++++--------------
arch/arm64/kernel/cpufeature.c | 23 +-
arch/arm64/kernel/entry-common.c | 36 +-
arch/arm64/kernel/proton-pack.c | 26 +-
arch/arm64/kernel/traps.c | 125 +++---
10 files changed, 396 insertions(+), 425 deletions(-)

--
2.34.1


2023-10-11 10:09:48

by Jinjie Ruan

[permalink] [raw]
Subject: [PATCH v5.15 11/15] arm64: armv8_deprecated: fold ops into insn_emulation

From: Mark Rutland <[email protected]>

commit b4453cc8a7ebbd45436a8cd3ffeaa069ceac146f upstream.

The code for emulating deprecated instructions has two related
structures: struct insn_emulation_ops and struct insn_emulation, where
each struct insn_emulation_ops is associated 1-1 with a struct
insn_emulation.

It would be simpler to combine the two into a single structure, removing
the need for (unconditional) dynamic allocation at boot time, and
simplifying some runtime pointer chasing.

This patch merges the two structures together.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: James Morse <[email protected]>
Cc: Joey Gouly <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Will Deacon <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Will Deacon <[email protected]>
Signed-off-by: Jinjie Ruan <[email protected]>
---
arch/arm64/kernel/armv8_deprecated.c | 76 ++++++++++++----------------
1 file changed, 33 insertions(+), 43 deletions(-)

diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
index c5da9d1e954a..494946942b7c 100644
--- a/arch/arm64/kernel/armv8_deprecated.c
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -41,16 +41,12 @@ enum legacy_insn_status {
INSN_OBSOLETE,
};

-struct insn_emulation_ops {
- const char *name;
- enum legacy_insn_status status;
- struct undef_hook *hooks;
- int (*set_hw_mode)(bool enable);
-};
-
struct insn_emulation {
- struct list_head node;
- struct insn_emulation_ops *ops;
+ const char *name;
+ struct list_head node;
+ enum legacy_insn_status status;
+ struct undef_hook *hooks;
+ int (*set_hw_mode)(bool enable);
int current_mode;
int min;
int max;
@@ -61,48 +57,48 @@ static int nr_insn_emulated __initdata;
static DEFINE_RAW_SPINLOCK(insn_emulation_lock);
static DEFINE_MUTEX(insn_emulation_mutex);

-static void register_emulation_hooks(struct insn_emulation_ops *ops)
+static void register_emulation_hooks(struct insn_emulation *insn)
{
struct undef_hook *hook;

- BUG_ON(!ops->hooks);
+ BUG_ON(!insn->hooks);

- for (hook = ops->hooks; hook->instr_mask; hook++)
+ for (hook = insn->hooks; hook->instr_mask; hook++)
register_undef_hook(hook);

- pr_notice("Registered %s emulation handler\n", ops->name);
+ pr_notice("Registered %s emulation handler\n", insn->name);
}

-static void remove_emulation_hooks(struct insn_emulation_ops *ops)
+static void remove_emulation_hooks(struct insn_emulation *insn)
{
struct undef_hook *hook;

- BUG_ON(!ops->hooks);
+ BUG_ON(!insn->hooks);

- for (hook = ops->hooks; hook->instr_mask; hook++)
+ for (hook = insn->hooks; hook->instr_mask; hook++)
unregister_undef_hook(hook);

- pr_notice("Removed %s emulation handler\n", ops->name);
+ pr_notice("Removed %s emulation handler\n", insn->name);
}

static void enable_insn_hw_mode(void *data)
{
struct insn_emulation *insn = (struct insn_emulation *)data;
- if (insn->ops->set_hw_mode)
- insn->ops->set_hw_mode(true);
+ if (insn->set_hw_mode)
+ insn->set_hw_mode(true);
}

static void disable_insn_hw_mode(void *data)
{
struct insn_emulation *insn = (struct insn_emulation *)data;
- if (insn->ops->set_hw_mode)
- insn->ops->set_hw_mode(false);
+ if (insn->set_hw_mode)
+ insn->set_hw_mode(false);
}

/* Run set_hw_mode(mode) on all active CPUs */
static int run_all_cpu_set_hw_mode(struct insn_emulation *insn, bool enable)
{
- if (!insn->ops->set_hw_mode)
+ if (!insn->set_hw_mode)
return -EINVAL;
if (enable)
on_each_cpu(enable_insn_hw_mode, (void *)insn, true);
@@ -126,9 +122,9 @@ static int run_all_insn_set_hw_mode(unsigned int cpu)
raw_spin_lock_irqsave(&insn_emulation_lock, flags);
list_for_each_entry(insn, &insn_emulation, node) {
bool enable = (insn->current_mode == INSN_HW);
- if (insn->ops->set_hw_mode && insn->ops->set_hw_mode(enable)) {
+ if (insn->set_hw_mode && insn->set_hw_mode(enable)) {
pr_warn("CPU[%u] cannot support the emulation of %s",
- cpu, insn->ops->name);
+ cpu, insn->name);
rc = -EINVAL;
}
}
@@ -145,11 +141,11 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
case INSN_UNDEF: /* Nothing to be done */
break;
case INSN_EMULATE:
- remove_emulation_hooks(insn->ops);
+ remove_emulation_hooks(insn);
break;
case INSN_HW:
if (!run_all_cpu_set_hw_mode(insn, false))
- pr_notice("Disabled %s support\n", insn->ops->name);
+ pr_notice("Disabled %s support\n", insn->name);
break;
}

@@ -157,31 +153,25 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
case INSN_UNDEF:
break;
case INSN_EMULATE:
- register_emulation_hooks(insn->ops);
+ register_emulation_hooks(insn);
break;
case INSN_HW:
ret = run_all_cpu_set_hw_mode(insn, true);
if (!ret)
- pr_notice("Enabled %s support\n", insn->ops->name);
+ pr_notice("Enabled %s support\n", insn->name);
break;
}

return ret;
}

-static void __init register_insn_emulation(struct insn_emulation_ops *ops)
+static void __init register_insn_emulation(struct insn_emulation *insn)
{
unsigned long flags;
- struct insn_emulation *insn;
-
- insn = kzalloc(sizeof(*insn), GFP_KERNEL);
- if (!insn)
- return;

- insn->ops = ops;
insn->min = INSN_UNDEF;

- switch (ops->status) {
+ switch (insn->status) {
case INSN_DEPRECATED:
insn->current_mode = INSN_EMULATE;
/* Disable the HW mode if it was turned on at early boot time */
@@ -247,7 +237,7 @@ static void __init register_insn_emulation_sysctl(void)
sysctl->mode = 0644;
sysctl->maxlen = sizeof(int);

- sysctl->procname = insn->ops->name;
+ sysctl->procname = insn->name;
sysctl->data = &insn->current_mode;
sysctl->extra1 = &insn->min;
sysctl->extra2 = &insn->max;
@@ -451,7 +441,7 @@ static struct undef_hook swp_hooks[] = {
{ }
};

-static struct insn_emulation_ops swp_ops = {
+static struct insn_emulation insn_swp = {
.name = "swp",
.status = INSN_OBSOLETE,
.hooks = swp_hooks,
@@ -538,7 +528,7 @@ static struct undef_hook cp15_barrier_hooks[] = {
{ }
};

-static struct insn_emulation_ops cp15_barrier_ops = {
+static struct insn_emulation insn_cp15_barrier = {
.name = "cp15_barrier",
.status = INSN_DEPRECATED,
.hooks = cp15_barrier_hooks,
@@ -611,7 +601,7 @@ static struct undef_hook setend_hooks[] = {
{}
};

-static struct insn_emulation_ops setend_ops = {
+static struct insn_emulation insn_setend = {
.name = "setend",
.status = INSN_DEPRECATED,
.hooks = setend_hooks,
@@ -625,14 +615,14 @@ static struct insn_emulation_ops setend_ops = {
static int __init armv8_deprecated_init(void)
{
if (IS_ENABLED(CONFIG_SWP_EMULATION))
- register_insn_emulation(&swp_ops);
+ register_insn_emulation(&insn_swp);

if (IS_ENABLED(CONFIG_CP15_BARRIER_EMULATION))
- register_insn_emulation(&cp15_barrier_ops);
+ register_insn_emulation(&insn_cp15_barrier);

if (IS_ENABLED(CONFIG_SETEND_EMULATION)) {
if (system_supports_mixed_endian_el0())
- register_insn_emulation(&setend_ops);
+ register_insn_emulation(&insn_setend);
else
pr_info("setend instruction emulation is not supported on this system\n");
}
--
2.34.1

2023-10-11 10:09:53

by Jinjie Ruan

[permalink] [raw]
Subject: [PATCH v5.15 15/15] arm64: armv8_deprecated: fix unused-function error

From: Ren Zhijie <[email protected]>

commit 223d3a0d30b6e9f979f5642e430e1753d3e29f89 upstream.

If CONFIG_SWP_EMULATION is not set and
CONFIG_CP15_BARRIER_EMULATION is not set,
aarch64-linux-gnu complained about unused-function :

arch/arm64/kernel/armv8_deprecated.c:67:21: error: ‘aarch32_check_condition’ defined but not used [-Werror=unused-function]
static unsigned int aarch32_check_condition(u32 opcode, u32 psr)
^~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

To fix this warning, modify aarch32_check_condition() with __maybe_unused.

Fixes: 0c5f416219da ("arm64: armv8_deprecated: move aarch32 helper earlier")
Signed-off-by: Ren Zhijie <[email protected]>
Acked-by: Mark Rutland <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Will Deacon <[email protected]>
Signed-off-by: Jinjie Ruan <[email protected]>
---
arch/arm64/kernel/armv8_deprecated.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
index 758008020b59..91eabe56093d 100644
--- a/arch/arm64/kernel/armv8_deprecated.c
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -64,7 +64,7 @@ struct insn_emulation {

#define ARM_OPCODE_CONDITION_UNCOND 0xf

-static unsigned int aarch32_check_condition(u32 opcode, u32 psr)
+static unsigned int __maybe_unused aarch32_check_condition(u32 opcode, u32 psr)
{
u32 cc_bits = opcode >> 28;

--
2.34.1

2023-10-11 10:10:32

by Jinjie Ruan

[permalink] [raw]
Subject: [PATCH v5.15 02/15] arm64: die(): pass 'err' as long

From: Mark Rutland <[email protected]>

commit 18906ff9af6517c20763ed63dab602a4150794f7 upstream.

Recently, we reworked a lot of code to consistentlt pass ESR_ELx as a
64-bit quantity. However, we missed that this can be passed into die()
and __die() as the 'err' parameter where it is truncated to a 32-bit
int.

As notify_die() already takes 'err' as a long, this patch changes die()
and __die() to also take 'err' as a long, ensuring that the full value
of ESR_ELx is retained.

At the same time, die() is updated to consistently log 'err' as a
zero-padded 64-bit quantity.

Subsequent patches will pass the ESR_ELx value to die() for a number of
exceptions.

Signed-off-by: Mark Rutland <[email protected]>
Reviewed-by: Mark Brown <[email protected]>
Reviewed-by: Anshuman Khandual <[email protected]>
Cc: Alexandru Elisei <[email protected]>
Cc: Amit Daniel Kachhap <[email protected]>
Cc: James Morse <[email protected]>
Cc: Will Deacon <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Catalin Marinas <[email protected]>
Signed-off-by: Jinjie Ruan <[email protected]>
---
arch/arm64/include/asm/system_misc.h | 2 +-
arch/arm64/kernel/traps.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/system_misc.h b/arch/arm64/include/asm/system_misc.h
index 0eb7709422e2..c34344256762 100644
--- a/arch/arm64/include/asm/system_misc.h
+++ b/arch/arm64/include/asm/system_misc.h
@@ -18,7 +18,7 @@

struct pt_regs;

-void die(const char *msg, struct pt_regs *regs, int err);
+void die(const char *msg, struct pt_regs *regs, long err);

struct siginfo;
void arm64_notify_die(const char *str, struct pt_regs *regs,
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 79e105713706..fcf1a306e094 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -181,12 +181,12 @@ static void dump_kernel_instr(const char *lvl, struct pt_regs *regs)

#define S_SMP " SMP"

-static int __die(const char *str, int err, struct pt_regs *regs)
+static int __die(const char *str, long err, struct pt_regs *regs)
{
static int die_counter;
int ret;

- pr_emerg("Internal error: %s: %x [#%d]" S_PREEMPT S_SMP "\n",
+ pr_emerg("Internal error: %s: %016lx [#%d]" S_PREEMPT S_SMP "\n",
str, err, ++die_counter);

/* trap and error numbers are mostly meaningless on ARM */
@@ -207,7 +207,7 @@ static DEFINE_RAW_SPINLOCK(die_lock);
/*
* This function is protected against re-entrancy.
*/
-void die(const char *str, struct pt_regs *regs, int err)
+void die(const char *str, struct pt_regs *regs, long err)
{
int ret;
unsigned long flags;
--
2.34.1

2023-10-11 10:11:34

by Jinjie Ruan

[permalink] [raw]
Subject: [PATCH v5.15 07/15] arm64: split EL0/EL1 UNDEF handlers

From: Mark Rutland <[email protected]>

commit 61d64a376ea80f9097e7ea599bcd68671b836dc6 upstream.

In general, exceptions taken from EL1 need to be handled separately from
exceptions taken from EL0, as the logic to handle the two cases can be
significantly divergent, and exceptions taken from EL1 typically have
more stringent requirements on locking and instrumentation.

Subsequent patches will rework the way EL1 UNDEFs are handled in order
to address longstanding soundness issues with instrumentation and RCU.
In preparation for that rework, this patch splits the existing
do_undefinstr() handler into separate do_el0_undef() and do_el1_undef()
handlers.

Prior to this patch, do_undefinstr() was marked with NOKPROBE_SYMBOL(),
preventing instrumentation via kprobes. However, do_undefinstr() invokes
other code which can be instrumented, and:

* For UNDEFINED exceptions taken from EL0, there is no risk of recursion
within kprobes. Therefore it is safe for do_el0_undef to be
instrumented with kprobes, and it does not need to be marked with
NOKPROBE_SYMBOL().

* For UNDEFINED exceptions taken from EL1, either:

(a) The exception is has been taken when manipulating SSBS; these cases
are limited and do not occur within code that can be invoked
recursively via kprobes. Hence, in these cases instrumentation
with kprobes is benign.

(b) The exception has been taken for an unknown reason, as other than
manipulating SSBS we do not expect to take UNDEFINED exceptions
from EL1. Any handling of these exception is best-effort.

... and in either case, marking do_el1_undef() with NOKPROBE_SYMBOL()
isn't sufficient to prevent recursion via kprobes as functions it
calls (including die()) are instrumentable via kprobes.

Hence, it's not worthwhile to mark do_el1_undef() with
NOKPROBE_SYMBOL(). The same applies to do_el1_bti() and do_el1_fpac(),
so their NOKPROBE_SYMBOL() annotations are also removed.

Aside from the new instrumentability, there should be no functional
change as a result of this patch.

Signed-off-by: Mark Rutland <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: James Morse <[email protected]>
Cc: Joey Gouly <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Will Deacon <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Will Deacon <[email protected]>
Signed-off-by: Jinjie Ruan <[email protected]>
---
arch/arm64/include/asm/exception.h | 3 ++-
arch/arm64/kernel/entry-common.c | 4 ++--
arch/arm64/kernel/traps.c | 22 ++++++++++++----------
3 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index 7466239adac0..515ebe24fd44 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h
@@ -58,7 +58,8 @@ asmlinkage void call_on_irq_stack(struct pt_regs *regs,
asmlinkage void asm_exit_to_user_mode(struct pt_regs *regs);

void do_mem_abort(unsigned long far, unsigned long esr, struct pt_regs *regs);
-void do_undefinstr(struct pt_regs *regs, unsigned long esr);
+void do_el0_undef(struct pt_regs *regs, unsigned long esr);
+void do_el1_undef(struct pt_regs *regs, unsigned long esr);
void do_el0_bti(struct pt_regs *regs);
void do_el1_bti(struct pt_regs *regs, unsigned long esr);
void do_debug_exception(unsigned long addr_if_watchpoint, unsigned long esr,
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index 31bdaccabd29..864423297c00 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -375,7 +375,7 @@ static void noinstr el1_undef(struct pt_regs *regs, unsigned long esr)
{
enter_from_kernel_mode(regs);
local_daif_inherit(regs);
- do_undefinstr(regs, esr);
+ do_el1_undef(regs, esr);
local_daif_mask();
exit_to_kernel_mode(regs);
}
@@ -570,7 +570,7 @@ static void noinstr el0_undef(struct pt_regs *regs, unsigned long esr)
{
enter_from_user_mode(regs);
local_daif_restore(DAIF_PROCCTX);
- do_undefinstr(regs, esr);
+ do_el0_undef(regs, esr);
exit_to_user_mode(regs);
}

diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 2ff0c8ddcacf..2999ccf4c117 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -486,7 +486,7 @@ void arm64_notify_segfault(unsigned long addr)
force_signal_inject(SIGSEGV, code, addr, 0);
}

-void do_undefinstr(struct pt_regs *regs, unsigned long esr)
+void do_el0_undef(struct pt_regs *regs, unsigned long esr)
{
/* check for AArch32 breakpoint instructions */
if (!aarch32_break_handler(regs))
@@ -495,12 +495,16 @@ void do_undefinstr(struct pt_regs *regs, unsigned long esr)
if (call_undef_hook(regs) == 0)
return;

- if (!user_mode(regs))
- die("Oops - Undefined instruction", regs, esr);
-
force_signal_inject(SIGILL, ILL_ILLOPC, regs->pc, 0);
}
-NOKPROBE_SYMBOL(do_undefinstr);
+
+void do_el1_undef(struct pt_regs *regs, unsigned long esr)
+{
+ if (call_undef_hook(regs) == 0)
+ return;
+
+ die("Oops - Undefined instruction", regs, esr);
+}

void do_el0_bti(struct pt_regs *regs)
{
@@ -511,7 +515,6 @@ void do_el1_bti(struct pt_regs *regs, unsigned long esr)
{
die("Oops - BTI", regs, esr);
}
-NOKPROBE_SYMBOL(do_el1_bti);

void do_el0_fpac(struct pt_regs *regs, unsigned long esr)
{
@@ -526,7 +529,6 @@ void do_el1_fpac(struct pt_regs *regs, unsigned long esr)
*/
die("Oops - FPAC", regs, esr);
}
-NOKPROBE_SYMBOL(do_el1_fpac);

#define __user_cache_maint(insn, address, res) \
if (address >= user_addr_max()) { \
@@ -763,7 +765,7 @@ void do_el0_cp15(unsigned long esr, struct pt_regs *regs)
hook_base = cp15_64_hooks;
break;
default:
- do_undefinstr(regs, esr);
+ do_el0_undef(regs, esr);
return;
}

@@ -778,7 +780,7 @@ void do_el0_cp15(unsigned long esr, struct pt_regs *regs)
* EL0. Fall back to our usual undefined instruction handler
* so that we handle these consistently.
*/
- do_undefinstr(regs, esr);
+ do_el0_undef(regs, esr);
}
#endif

@@ -797,7 +799,7 @@ void do_el0_sys(unsigned long esr, struct pt_regs *regs)
* back to our usual undefined instruction handler so that we handle
* these consistently.
*/
- do_undefinstr(regs, esr);
+ do_el0_undef(regs, esr);
}

static const char *esr_class_str[] = {
--
2.34.1

2023-10-11 10:18:24

by Jinjie Ruan

[permalink] [raw]
Subject: [PATCH v5.15 10/15] arm64: rework EL0 MRS emulation

From: Mark Rutland <[email protected]>

commit f5962add74b61f8ae31c6311f75ca35d7e1d2d8f upstream.

On CPUs without FEAT_IDST, ID register emulation is slower than it needs
to be, as all threads contend for the same lock to perform the
emulation. This patch reworks the emulation to avoid this unnecessary
contention.

On CPUs with FEAT_IDST (which is mandatory from ARMv8.4 onwards), EL0
accesses to ID registers result in a SYS trap, and emulation of these is
handled with a sys64_hook. These hooks are statically allocated, and no
locking is required to iterate through the hooks and perform the
emulation, allowing emulation to occur in parallel with no contention.

On CPUs without FEAT_IDST, EL0 accesses to ID registers result in an
UNDEFINED exception, and emulation of these accesses is handled with an
undef_hook. When an EL0 MRS instruction is trapped to EL1, the kernel
finds the relevant handler by iterating through all of the undef_hooks,
requiring undef_lock to be held during this lookup.

This locking is only required to safely traverse the list of undef_hooks
(as it can be concurrently modified), and the actual emulation of the
MRS does not require any mutual exclusion. This locking is an
unfortunate bottleneck, especially given that MRS emulation is enabled
unconditionally and is never disabled.

This patch reworks the non-FEAT_IDST MRS emulation logic so that it can
be invoked directly from do_el0_undef(). This removes the bottleneck,
allowing MRS traps to be handled entirely in parallel, and is a stepping
stone to making all of the undef_hooks lock-free.

I've tested this in a 64-vCPU VM on a 64-CPU ThunderX2 host, with a
benchmark which spawns a number of threads which each try to read
ID_AA64ISAR0_EL1 1000000 times. This is vastly more contention than will
ever be seen in realistic usage, but clearly demonstrates the removal of
the bottleneck:

| Threads || Time (seconds) |
| || Before || After |
| || Real | System || Real | System |
|---------++--------+---------++--------+---------|
| 1 || 0.29 | 0.20 || 0.24 | 0.12 |
| 2 || 0.35 | 0.51 || 0.23 | 0.27 |
| 4 || 1.08 | 3.87 || 0.24 | 0.56 |
| 8 || 4.31 | 33.60 || 0.24 | 1.11 |
| 16 || 9.47 | 149.39 || 0.23 | 2.15 |
| 32 || 19.07 | 605.27 || 0.24 | 4.38 |
| 64 || 65.40 | 3609.09 || 0.33 | 11.27 |

Aside from the speedup, there should be no functional change as a result
of this patch.

Signed-off-by: Mark Rutland <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: James Morse <[email protected]>
Cc: Joey Gouly <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Will Deacon <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Will Deacon <[email protected]>
Signed-off-by: Jinjie Ruan <[email protected]>
---
arch/arm64/include/asm/cpufeature.h | 3 ++-
arch/arm64/kernel/cpufeature.c | 23 +++++------------------
arch/arm64/kernel/traps.c | 3 +++
3 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index a77b5f49b3a6..72aaf7b92b31 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -808,7 +808,8 @@ static inline bool system_supports_tlb_range(void)
cpus_have_const_cap(ARM64_HAS_TLB_RANGE);
}

-extern int do_emulate_mrs(struct pt_regs *regs, u32 sys_reg, u32 rt);
+int do_emulate_mrs(struct pt_regs *regs, u32 sys_reg, u32 rt);
+bool try_emulate_mrs(struct pt_regs *regs, u32 isn);

static inline u32 id_aa64mmfr0_parange_to_phys_shift(int parange)
{
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index d4ee345ff429..f17d6cdea260 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -3106,35 +3106,22 @@ int do_emulate_mrs(struct pt_regs *regs, u32 sys_reg, u32 rt)
return rc;
}

-static int emulate_mrs(struct pt_regs *regs, u32 insn)
+bool try_emulate_mrs(struct pt_regs *regs, u32 insn)
{
u32 sys_reg, rt;

+ if (compat_user_mode(regs) || !aarch64_insn_is_mrs(insn))
+ return false;
+
/*
* sys_reg values are defined as used in mrs/msr instruction.
* shift the imm value to get the encoding.
*/
sys_reg = (u32)aarch64_insn_decode_immediate(AARCH64_INSN_IMM_16, insn) << 5;
rt = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RT, insn);
- return do_emulate_mrs(regs, sys_reg, rt);
+ return do_emulate_mrs(regs, sys_reg, rt) == 0;
}

-static struct undef_hook mrs_hook = {
- .instr_mask = 0xffff0000,
- .instr_val = 0xd5380000,
- .pstate_mask = PSR_AA32_MODE_MASK,
- .pstate_val = PSR_MODE_EL0t,
- .fn = emulate_mrs,
-};
-
-static int __init enable_mrs_emulation(void)
-{
- register_undef_hook(&mrs_hook);
- return 0;
-}
-
-core_initcall(enable_mrs_emulation);
-
enum mitigation_state arm64_get_meltdown_state(void)
{
if (__meltdown_safe)
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index e943b7c46d86..1f9285a35560 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -499,6 +499,9 @@ void do_el0_undef(struct pt_regs *regs, unsigned long esr)
if (user_insn_read(regs, &insn))
goto out_err;

+ if (try_emulate_mrs(regs, insn))
+ return;
+
if (call_undef_hook(regs, insn) == 0)
return;

--
2.34.1

2023-10-11 10:18:28

by Jinjie Ruan

[permalink] [raw]
Subject: [PATCH v5.15 13/15] arm64: armv8_deprecated: move aarch32 helper earlier

From: Mark Rutland <[email protected]>

commit 0c5f416219da3795dc8b33e5bb7865a6b3c4e55c upstream.

Subsequent patches will rework the logic in armv8_deprecated.c.

In preparation for subsequent changes, this patch moves some shared logic
earlier in the file. This will make subsequent diffs simpler and easier to
read.

At the same time, drop the `__kprobes` annotation from
aarch32_check_condition(), as this is only used for traps from compat
userspace, and has no risk of recursion within kprobes. As this is the
last kprobes annotation in armve8_deprecated.c, we no longer need to
include <asm/kprobes.h>.

Signed-off-by: Mark Rutland <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: James Morse <[email protected]>
Cc: Joey Gouly <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Will Deacon <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Will Deacon <[email protected]>
Signed-off-by: Jinjie Ruan <[email protected]>
---
arch/arm64/kernel/armv8_deprecated.c | 39 ++++++++++++++--------------
1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
index e52ae01b4805..6555bceff54b 100644
--- a/arch/arm64/kernel/armv8_deprecated.c
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -17,7 +17,6 @@
#include <asm/sysreg.h>
#include <asm/system_misc.h>
#include <asm/traps.h>
-#include <asm/kprobes.h>

#define CREATE_TRACE_POINTS
#include "trace-events-emulation.h"
@@ -52,6 +51,25 @@ struct insn_emulation {
int max;
};

+#define ARM_OPCODE_CONDTEST_FAIL 0
+#define ARM_OPCODE_CONDTEST_PASS 1
+#define ARM_OPCODE_CONDTEST_UNCOND 2
+
+#define ARM_OPCODE_CONDITION_UNCOND 0xf
+
+static unsigned int aarch32_check_condition(u32 opcode, u32 psr)
+{
+ u32 cc_bits = opcode >> 28;
+
+ if (cc_bits != ARM_OPCODE_CONDITION_UNCOND) {
+ if ((*aarch32_opcode_cond_checks[cc_bits])(psr))
+ return ARM_OPCODE_CONDTEST_PASS;
+ else
+ return ARM_OPCODE_CONDTEST_FAIL;
+ }
+ return ARM_OPCODE_CONDTEST_UNCOND;
+}
+
/*
* Implement emulation of the SWP/SWPB instructions using load-exclusive and
* store-exclusive.
@@ -138,25 +156,6 @@ static int emulate_swpX(unsigned int address, unsigned int *data,
return res;
}

-#define ARM_OPCODE_CONDTEST_FAIL 0
-#define ARM_OPCODE_CONDTEST_PASS 1
-#define ARM_OPCODE_CONDTEST_UNCOND 2
-
-#define ARM_OPCODE_CONDITION_UNCOND 0xf
-
-static unsigned int __kprobes aarch32_check_condition(u32 opcode, u32 psr)
-{
- u32 cc_bits = opcode >> 28;
-
- if (cc_bits != ARM_OPCODE_CONDITION_UNCOND) {
- if ((*aarch32_opcode_cond_checks[cc_bits])(psr))
- return ARM_OPCODE_CONDTEST_PASS;
- else
- return ARM_OPCODE_CONDTEST_FAIL;
- }
- return ARM_OPCODE_CONDTEST_UNCOND;
-}
-
/*
* swp_handler logs the id of calling process, dissects the instruction, sanity
* checks the memory location, calls emulate_swpX for the actual operation and
--
2.34.1

2023-10-11 10:18:39

by Jinjie Ruan

[permalink] [raw]
Subject: [PATCH v5.15 03/15] arm64: consistently pass ESR_ELx to die()

From: Mark Rutland <[email protected]>

commit 0f2cb928a1547ae8f89e80a4b8df2c6c02ae5f96 upstream.

Currently, bug_handler() and kasan_handler() call die() with '0' as the
'err' value, whereas die_kernel_fault() passes the ESR_ELx value.

For consistency, this patch ensures we always pass the ESR_ELx value to
die(). As this is only called for exceptions taken from kernel mode,
there should be no user-visible change as a result of this patch.

For UNDEFINED exceptions, I've had to modify do_undefinstr() and its
callers to pass the ESR_ELx value. In all cases the ESR_ELx value had
already been read and was available.

Signed-off-by: Mark Rutland <[email protected]>
Cc: Mark Brown <[email protected]>
Cc: Alexandru Elisei <[email protected]>
Cc: Amit Daniel Kachhap <[email protected]>
Cc: James Morse <[email protected]>
Cc: Will Deacon <[email protected]>
Reviewed-by: Anshuman Khandual <[email protected]>
Reviewed-by: Mark Brown <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Catalin Marinas <[email protected]>
Signed-off-by: Jinjie Ruan <[email protected]>
---
arch/arm64/include/asm/exception.h | 2 +-
arch/arm64/kernel/entry-common.c | 14 +++++++-------
arch/arm64/kernel/traps.c | 14 +++++++-------
3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index 0e6535aa78c2..babbe2db8010 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h
@@ -58,7 +58,7 @@ asmlinkage void call_on_irq_stack(struct pt_regs *regs,
asmlinkage void asm_exit_to_user_mode(struct pt_regs *regs);

void do_mem_abort(unsigned long far, unsigned long esr, struct pt_regs *regs);
-void do_undefinstr(struct pt_regs *regs);
+void do_undefinstr(struct pt_regs *regs, unsigned long esr);
void do_bti(struct pt_regs *regs);
void do_debug_exception(unsigned long addr_if_watchpoint, unsigned long esr,
struct pt_regs *regs);
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index fc91dad1579a..4dbdadc5daa1 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -371,11 +371,11 @@ static void noinstr el1_pc(struct pt_regs *regs, unsigned long esr)
exit_to_kernel_mode(regs);
}

-static void noinstr el1_undef(struct pt_regs *regs)
+static void noinstr el1_undef(struct pt_regs *regs, unsigned long esr)
{
enter_from_kernel_mode(regs);
local_daif_inherit(regs);
- do_undefinstr(regs);
+ do_undefinstr(regs, esr);
local_daif_mask();
exit_to_kernel_mode(regs);
}
@@ -417,7 +417,7 @@ asmlinkage void noinstr el1h_64_sync_handler(struct pt_regs *regs)
break;
case ESR_ELx_EC_SYS64:
case ESR_ELx_EC_UNKNOWN:
- el1_undef(regs);
+ el1_undef(regs, esr);
break;
case ESR_ELx_EC_BREAKPT_CUR:
case ESR_ELx_EC_SOFTSTP_CUR:
@@ -554,11 +554,11 @@ static void noinstr el0_sp(struct pt_regs *regs, unsigned long esr)
exit_to_user_mode(regs);
}

-static void noinstr el0_undef(struct pt_regs *regs)
+static void noinstr el0_undef(struct pt_regs *regs, unsigned long esr)
{
enter_from_user_mode(regs);
local_daif_restore(DAIF_PROCCTX);
- do_undefinstr(regs);
+ do_undefinstr(regs, esr);
exit_to_user_mode(regs);
}

@@ -639,7 +639,7 @@ asmlinkage void noinstr el0t_64_sync_handler(struct pt_regs *regs)
el0_pc(regs, esr);
break;
case ESR_ELx_EC_UNKNOWN:
- el0_undef(regs);
+ el0_undef(regs, esr);
break;
case ESR_ELx_EC_BTI:
el0_bti(regs);
@@ -755,7 +755,7 @@ asmlinkage void noinstr el0t_32_sync_handler(struct pt_regs *regs)
case ESR_ELx_EC_CP14_MR:
case ESR_ELx_EC_CP14_LS:
case ESR_ELx_EC_CP14_64:
- el0_undef(regs);
+ el0_undef(regs, esr);
break;
case ESR_ELx_EC_CP15_32:
case ESR_ELx_EC_CP15_64:
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index fcf1a306e094..591eb1a44d30 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -486,7 +486,7 @@ void arm64_notify_segfault(unsigned long addr)
force_signal_inject(SIGSEGV, code, addr, 0);
}

-void do_undefinstr(struct pt_regs *regs)
+void do_undefinstr(struct pt_regs *regs, unsigned long esr)
{
/* check for AArch32 breakpoint instructions */
if (!aarch32_break_handler(regs))
@@ -496,7 +496,7 @@ void do_undefinstr(struct pt_regs *regs)
return;

if (!user_mode(regs))
- die("Oops - Undefined instruction", regs, 0);
+ die("Oops - Undefined instruction", regs, esr);

force_signal_inject(SIGILL, ILL_ILLOPC, regs->pc, 0);
}
@@ -755,7 +755,7 @@ void do_cp15instr(unsigned long esr, struct pt_regs *regs)
hook_base = cp15_64_hooks;
break;
default:
- do_undefinstr(regs);
+ do_undefinstr(regs, esr);
return;
}

@@ -770,7 +770,7 @@ void do_cp15instr(unsigned long esr, struct pt_regs *regs)
* EL0. Fall back to our usual undefined instruction handler
* so that we handle these consistently.
*/
- do_undefinstr(regs);
+ do_undefinstr(regs, esr);
}
NOKPROBE_SYMBOL(do_cp15instr);
#endif
@@ -790,7 +790,7 @@ void do_sysinstr(unsigned long esr, struct pt_regs *regs)
* back to our usual undefined instruction handler so that we handle
* these consistently.
*/
- do_undefinstr(regs);
+ do_undefinstr(regs, esr);
}
NOKPROBE_SYMBOL(do_sysinstr);

@@ -966,7 +966,7 @@ static int bug_handler(struct pt_regs *regs, unsigned long esr)
{
switch (report_bug(regs->pc, regs)) {
case BUG_TRAP_TYPE_BUG:
- die("Oops - BUG", regs, 0);
+ die("Oops - BUG", regs, esr);
break;

case BUG_TRAP_TYPE_WARN:
@@ -1034,7 +1034,7 @@ static int kasan_handler(struct pt_regs *regs, unsigned long esr)
* This is something that might be fixed at some point in the future.
*/
if (!recover)
- die("Oops - KASAN", regs, 0);
+ die("Oops - KASAN", regs, esr);

/* If thread survives, skip over the brk instruction and continue: */
arm64_skip_faulting_instruction(regs, AARCH64_INSN_SIZE);
--
2.34.1

2023-10-11 10:18:42

by Jinjie Ruan

[permalink] [raw]
Subject: [PATCH v5.15 06/15] arm64: allow kprobes on EL0 handlers

From: Mark Rutland <[email protected]>

commit b3a0c010e900a9f89dcd99f10bd8f7538d21b0a9 upstream.

Currently do_sysinstr() and do_cp15instr() are marked with
NOKPROBE_SYMBOL(). However, these are only called for exceptions taken
from EL0, and there is no risk of recursion in kprobes, so this is not
necessary.

Remove the NOKPROBE_SYMBOL() annotation, and rename the two functions to
more clearly indicate that these are solely for exceptions taken from
EL0, better matching the names used by the lower level entry points in
entry-common.c.

Signed-off-by: Mark Rutland <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: James Morse <[email protected]>
Cc: Joey Gouly <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Will Deacon <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Will Deacon <[email protected]>
Signed-off-by: Jinjie Ruan <[email protected]>
---
arch/arm64/include/asm/exception.h | 4 ++--
arch/arm64/kernel/entry-common.c | 4 ++--
arch/arm64/kernel/traps.c | 6 ++----
3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index 81b559a780e8..7466239adac0 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h
@@ -66,10 +66,10 @@ void do_debug_exception(unsigned long addr_if_watchpoint, unsigned long esr,
void do_fpsimd_acc(unsigned long esr, struct pt_regs *regs);
void do_sve_acc(unsigned long esr, struct pt_regs *regs);
void do_fpsimd_exc(unsigned long esr, struct pt_regs *regs);
-void do_sysinstr(unsigned long esr, struct pt_regs *regs);
+void do_el0_sys(unsigned long esr, struct pt_regs *regs);
void do_sp_pc_abort(unsigned long addr, unsigned long esr, struct pt_regs *regs);
void bad_el0_sync(struct pt_regs *regs, int reason, unsigned long esr);
-void do_cp15instr(unsigned long esr, struct pt_regs *regs);
+void do_el0_cp15(unsigned long esr, struct pt_regs *regs);
void do_el0_svc(struct pt_regs *regs);
void do_el0_svc_compat(struct pt_regs *regs);
void do_el0_fpac(struct pt_regs *regs, unsigned long esr);
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index 6caab4453b11..31bdaccabd29 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -541,7 +541,7 @@ static void noinstr el0_sys(struct pt_regs *regs, unsigned long esr)
{
enter_from_user_mode(regs);
local_daif_restore(DAIF_PROCCTX);
- do_sysinstr(esr, regs);
+ do_el0_sys(esr, regs);
exit_to_user_mode(regs);
}

@@ -728,7 +728,7 @@ static void noinstr el0_cp15(struct pt_regs *regs, unsigned long esr)
{
enter_from_user_mode(regs);
local_daif_restore(DAIF_PROCCTX);
- do_cp15instr(esr, regs);
+ do_el0_cp15(esr, regs);
exit_to_user_mode(regs);
}

diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index ae1844e9daf0..2ff0c8ddcacf 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -742,7 +742,7 @@ static const struct sys64_hook cp15_64_hooks[] = {
{},
};

-void do_cp15instr(unsigned long esr, struct pt_regs *regs)
+void do_el0_cp15(unsigned long esr, struct pt_regs *regs)
{
const struct sys64_hook *hook, *hook_base;

@@ -780,10 +780,9 @@ void do_cp15instr(unsigned long esr, struct pt_regs *regs)
*/
do_undefinstr(regs, esr);
}
-NOKPROBE_SYMBOL(do_cp15instr);
#endif

-void do_sysinstr(unsigned long esr, struct pt_regs *regs)
+void do_el0_sys(unsigned long esr, struct pt_regs *regs)
{
const struct sys64_hook *hook;

@@ -800,7 +799,6 @@ void do_sysinstr(unsigned long esr, struct pt_regs *regs)
*/
do_undefinstr(regs, esr);
}
-NOKPROBE_SYMBOL(do_sysinstr);

static const char *esr_class_str[] = {
[0 ... ESR_ELx_EC_MAX] = "UNRECOGNIZED EC",
--
2.34.1

2023-10-11 10:18:57

by Jinjie Ruan

[permalink] [raw]
Subject: [PATCH v5.15 01/15] arm64: report EL1 UNDEFs better

From: Mark Rutland <[email protected]>

commit b502c87d2a26c349acbc231ff2acd6f17147926b upstream.

If an UNDEFINED exception is taken from EL1, and do_undefinstr() doesn't
find any suitable undef_hook, it will call:

BUG_ON(!user_mode(regs))

... and the kernel will report a failure witin do_undefinstr() rather
than reporting the original context that the UNDEFINED exception was
taken from. The pt_regs and ESR value reported within the BUG() handler
will be from within do_undefinstr() and the code dump will be for the
BRK in BUG_ON(), which isn't sufficient to debug the cause of the
original exception.

This patch makes the reporting better by having do_undefinstr() call
die() directly in this case to report the original context from which
the UNDEFINED exception was taken.

Prior to this patch, an undefined instruction is reported as:

| kernel BUG at arch/arm64/kernel/traps.c:497!
| Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
| Modules linked in:
| CPU: 0 PID: 0 Comm: swapper Not tainted 5.19.0-rc3-00127-geff044f1b04e-dirty #3
| Hardware name: linux,dummy-virt (DT)
| pstate: 000000c5 (nzcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
| pc : do_undefinstr+0x28c/0x2ac
| lr : do_undefinstr+0x298/0x2ac
| sp : ffff800009f63bc0
| x29: ffff800009f63bc0 x28: ffff800009f73c00 x27: ffff800009644a70
| x26: ffff8000096778a8 x25: 0000000000000040 x24: 0000000000000000
| x23: 00000000800000c5 x22: ffff800009894060 x21: ffff800009f63d90
| x20: 0000000000000000 x19: ffff800009f63c40 x18: 0000000000000006
| x17: 0000000000403000 x16: 00000000bfbfd000 x15: ffff800009f63830
| x14: ffffffffffffffff x13: 0000000000000000 x12: 0000000000000019
| x11: 0101010101010101 x10: 0000000000161b98 x9 : 0000000000000000
| x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000000
| x5 : ffff800009f761d0 x4 : 0000000000000000 x3 : ffff80000a2b80f8
| x2 : 0000000000000000 x1 : ffff800009f73c00 x0 : 00000000800000c5
| Call trace:
| do_undefinstr+0x28c/0x2ac
| el1_undef+0x2c/0x4c
| el1h_64_sync_handler+0x84/0xd0
| el1h_64_sync+0x64/0x68
| setup_arch+0x550/0x598
| start_kernel+0x88/0x6ac
| __primary_switched+0xb8/0xc0
| Code: 17ffff95 a9425bf5 17ffffb8 a9025bf5 (d4210000)

With this patch applied, an undefined instruction is reported as:

| Internal error: Oops - Undefined instruction: 0 [#1] PREEMPT SMP
| Modules linked in:
| CPU: 0 PID: 0 Comm: swapper Not tainted 5.19.0-rc3-00128-gf27cfcc80e52-dirty #5
| Hardware name: linux,dummy-virt (DT)
| pstate: 800000c5 (Nzcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
| pc : setup_arch+0x550/0x598
| lr : setup_arch+0x50c/0x598
| sp : ffff800009f63d90
| x29: ffff800009f63d90 x28: 0000000081000200 x27: ffff800009644a70
| x26: ffff8000096778c8 x25: 0000000000000040 x24: 0000000000000000
| x23: 0000000000000100 x22: ffff800009f69a58 x21: ffff80000a2b80b8
| x20: 0000000000000000 x19: 0000000000000000 x18: 0000000000000006
| x17: 0000000000403000 x16: 00000000bfbfd000 x15: ffff800009f63830
| x14: ffffffffffffffff x13: 0000000000000000 x12: 0000000000000019
| x11: 0101010101010101 x10: 0000000000161b98 x9 : 0000000000000000
| x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000000
| x5 : 0000000000000008 x4 : 0000000000000010 x3 : 0000000000000000
| x2 : 0000000000000000 x1 : 0000000000000000 x0 : 0000000000000000
| Call trace:
| setup_arch+0x550/0x598
| start_kernel+0x88/0x6ac
| __primary_switched+0xb8/0xc0
| Code: b4000080 90ffed80 912ac000 97db745f (00000000)

Signed-off-by: Mark Rutland <[email protected]>
Reviewed-by: Mark Brown <[email protected]>
Cc: Alexandru Elisei <[email protected]>
Cc: Amit Daniel Kachhap <[email protected]>
Cc: James Morse <[email protected]>
Cc: Will Deacon <[email protected]>
Reviewed-by: Anshuman Khandual <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Catalin Marinas <[email protected]>
Signed-off-by: Jinjie Ruan <[email protected]>
---
arch/arm64/kernel/traps.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 21e69a991bc8..79e105713706 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -495,7 +495,9 @@ void do_undefinstr(struct pt_regs *regs)
if (call_undef_hook(regs) == 0)
return;

- BUG_ON(!user_mode(regs));
+ if (!user_mode(regs))
+ die("Oops - Undefined instruction", regs, 0);
+
force_signal_inject(SIGILL, ILL_ILLOPC, regs->pc, 0);
}
NOKPROBE_SYMBOL(do_undefinstr);
--
2.34.1

2023-10-11 10:19:26

by Jinjie Ruan

[permalink] [raw]
Subject: [PATCH v5.15 12/15] arm64: armv8_deprecated move emulation functions

From: Mark Rutland <[email protected]>

commit 25eeac0cfe7c97ade1be07340e11e7143aab57a6 upstream.

Subsequent patches will rework the logic in armv8_deprecated.c.

In preparation for subsequent changes, this patch moves the emulation
logic earlier in the file, and moves the infrastructure later in the
file. This will make subsequent diffs simpler and easier to read.

This is purely a move. There should be no functional change as a result
of this patch.

Signed-off-by: Mark Rutland <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: James Morse <[email protected]>
Cc: Joey Gouly <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Will Deacon <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Will Deacon <[email protected]>
Signed-off-by: Jinjie Ruan <[email protected]>
---
arch/arm64/kernel/armv8_deprecated.c | 394 +++++++++++++--------------
1 file changed, 197 insertions(+), 197 deletions(-)

diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
index 494946942b7c..e52ae01b4805 100644
--- a/arch/arm64/kernel/armv8_deprecated.c
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -52,203 +52,6 @@ struct insn_emulation {
int max;
};

-static LIST_HEAD(insn_emulation);
-static int nr_insn_emulated __initdata;
-static DEFINE_RAW_SPINLOCK(insn_emulation_lock);
-static DEFINE_MUTEX(insn_emulation_mutex);
-
-static void register_emulation_hooks(struct insn_emulation *insn)
-{
- struct undef_hook *hook;
-
- BUG_ON(!insn->hooks);
-
- for (hook = insn->hooks; hook->instr_mask; hook++)
- register_undef_hook(hook);
-
- pr_notice("Registered %s emulation handler\n", insn->name);
-}
-
-static void remove_emulation_hooks(struct insn_emulation *insn)
-{
- struct undef_hook *hook;
-
- BUG_ON(!insn->hooks);
-
- for (hook = insn->hooks; hook->instr_mask; hook++)
- unregister_undef_hook(hook);
-
- pr_notice("Removed %s emulation handler\n", insn->name);
-}
-
-static void enable_insn_hw_mode(void *data)
-{
- struct insn_emulation *insn = (struct insn_emulation *)data;
- if (insn->set_hw_mode)
- insn->set_hw_mode(true);
-}
-
-static void disable_insn_hw_mode(void *data)
-{
- struct insn_emulation *insn = (struct insn_emulation *)data;
- if (insn->set_hw_mode)
- insn->set_hw_mode(false);
-}
-
-/* Run set_hw_mode(mode) on all active CPUs */
-static int run_all_cpu_set_hw_mode(struct insn_emulation *insn, bool enable)
-{
- if (!insn->set_hw_mode)
- return -EINVAL;
- if (enable)
- on_each_cpu(enable_insn_hw_mode, (void *)insn, true);
- else
- on_each_cpu(disable_insn_hw_mode, (void *)insn, true);
- return 0;
-}
-
-/*
- * Run set_hw_mode for all insns on a starting CPU.
- * Returns:
- * 0 - If all the hooks ran successfully.
- * -EINVAL - At least one hook is not supported by the CPU.
- */
-static int run_all_insn_set_hw_mode(unsigned int cpu)
-{
- int rc = 0;
- unsigned long flags;
- struct insn_emulation *insn;
-
- raw_spin_lock_irqsave(&insn_emulation_lock, flags);
- list_for_each_entry(insn, &insn_emulation, node) {
- bool enable = (insn->current_mode == INSN_HW);
- if (insn->set_hw_mode && insn->set_hw_mode(enable)) {
- pr_warn("CPU[%u] cannot support the emulation of %s",
- cpu, insn->name);
- rc = -EINVAL;
- }
- }
- raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
- return rc;
-}
-
-static int update_insn_emulation_mode(struct insn_emulation *insn,
- enum insn_emulation_mode prev)
-{
- int ret = 0;
-
- switch (prev) {
- case INSN_UNDEF: /* Nothing to be done */
- break;
- case INSN_EMULATE:
- remove_emulation_hooks(insn);
- break;
- case INSN_HW:
- if (!run_all_cpu_set_hw_mode(insn, false))
- pr_notice("Disabled %s support\n", insn->name);
- break;
- }
-
- switch (insn->current_mode) {
- case INSN_UNDEF:
- break;
- case INSN_EMULATE:
- register_emulation_hooks(insn);
- break;
- case INSN_HW:
- ret = run_all_cpu_set_hw_mode(insn, true);
- if (!ret)
- pr_notice("Enabled %s support\n", insn->name);
- break;
- }
-
- return ret;
-}
-
-static void __init register_insn_emulation(struct insn_emulation *insn)
-{
- unsigned long flags;
-
- insn->min = INSN_UNDEF;
-
- switch (insn->status) {
- case INSN_DEPRECATED:
- insn->current_mode = INSN_EMULATE;
- /* Disable the HW mode if it was turned on at early boot time */
- run_all_cpu_set_hw_mode(insn, false);
- insn->max = INSN_HW;
- break;
- case INSN_OBSOLETE:
- insn->current_mode = INSN_UNDEF;
- insn->max = INSN_EMULATE;
- break;
- }
-
- raw_spin_lock_irqsave(&insn_emulation_lock, flags);
- list_add(&insn->node, &insn_emulation);
- nr_insn_emulated++;
- raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
-
- /* Register any handlers if required */
- update_insn_emulation_mode(insn, INSN_UNDEF);
-}
-
-static int emulation_proc_handler(struct ctl_table *table, int write,
- void *buffer, size_t *lenp,
- loff_t *ppos)
-{
- int ret = 0;
- struct insn_emulation *insn = container_of(table->data, struct insn_emulation, current_mode);
- enum insn_emulation_mode prev_mode = insn->current_mode;
-
- mutex_lock(&insn_emulation_mutex);
- ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
-
- if (ret || !write || prev_mode == insn->current_mode)
- goto ret;
-
- ret = update_insn_emulation_mode(insn, prev_mode);
- if (ret) {
- /* Mode change failed, revert to previous mode. */
- insn->current_mode = prev_mode;
- update_insn_emulation_mode(insn, INSN_UNDEF);
- }
-ret:
- mutex_unlock(&insn_emulation_mutex);
- return ret;
-}
-
-static void __init register_insn_emulation_sysctl(void)
-{
- unsigned long flags;
- int i = 0;
- struct insn_emulation *insn;
- struct ctl_table *insns_sysctl, *sysctl;
-
- insns_sysctl = kcalloc(nr_insn_emulated + 1, sizeof(*sysctl),
- GFP_KERNEL);
- if (!insns_sysctl)
- return;
-
- raw_spin_lock_irqsave(&insn_emulation_lock, flags);
- list_for_each_entry(insn, &insn_emulation, node) {
- sysctl = &insns_sysctl[i];
-
- sysctl->mode = 0644;
- sysctl->maxlen = sizeof(int);
-
- sysctl->procname = insn->name;
- sysctl->data = &insn->current_mode;
- sysctl->extra1 = &insn->min;
- sysctl->extra2 = &insn->max;
- sysctl->proc_handler = emulation_proc_handler;
- i++;
- }
- raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
-
- register_sysctl("abi", insns_sysctl);
-}
-
/*
* Implement emulation of the SWP/SWPB instructions using load-exclusive and
* store-exclusive.
@@ -608,6 +411,203 @@ static struct insn_emulation insn_setend = {
.set_hw_mode = setend_set_hw_mode,
};

+static LIST_HEAD(insn_emulation);
+static int nr_insn_emulated __initdata;
+static DEFINE_RAW_SPINLOCK(insn_emulation_lock);
+static DEFINE_MUTEX(insn_emulation_mutex);
+
+static void register_emulation_hooks(struct insn_emulation *insn)
+{
+ struct undef_hook *hook;
+
+ BUG_ON(!insn->hooks);
+
+ for (hook = insn->hooks; hook->instr_mask; hook++)
+ register_undef_hook(hook);
+
+ pr_notice("Registered %s emulation handler\n", insn->name);
+}
+
+static void remove_emulation_hooks(struct insn_emulation *insn)
+{
+ struct undef_hook *hook;
+
+ BUG_ON(!insn->hooks);
+
+ for (hook = insn->hooks; hook->instr_mask; hook++)
+ unregister_undef_hook(hook);
+
+ pr_notice("Removed %s emulation handler\n", insn->name);
+}
+
+static void enable_insn_hw_mode(void *data)
+{
+ struct insn_emulation *insn = (struct insn_emulation *)data;
+ if (insn->set_hw_mode)
+ insn->set_hw_mode(true);
+}
+
+static void disable_insn_hw_mode(void *data)
+{
+ struct insn_emulation *insn = (struct insn_emulation *)data;
+ if (insn->set_hw_mode)
+ insn->set_hw_mode(false);
+}
+
+/* Run set_hw_mode(mode) on all active CPUs */
+static int run_all_cpu_set_hw_mode(struct insn_emulation *insn, bool enable)
+{
+ if (!insn->set_hw_mode)
+ return -EINVAL;
+ if (enable)
+ on_each_cpu(enable_insn_hw_mode, (void *)insn, true);
+ else
+ on_each_cpu(disable_insn_hw_mode, (void *)insn, true);
+ return 0;
+}
+
+/*
+ * Run set_hw_mode for all insns on a starting CPU.
+ * Returns:
+ * 0 - If all the hooks ran successfully.
+ * -EINVAL - At least one hook is not supported by the CPU.
+ */
+static int run_all_insn_set_hw_mode(unsigned int cpu)
+{
+ int rc = 0;
+ unsigned long flags;
+ struct insn_emulation *insn;
+
+ raw_spin_lock_irqsave(&insn_emulation_lock, flags);
+ list_for_each_entry(insn, &insn_emulation, node) {
+ bool enable = (insn->current_mode == INSN_HW);
+ if (insn->set_hw_mode && insn->set_hw_mode(enable)) {
+ pr_warn("CPU[%u] cannot support the emulation of %s",
+ cpu, insn->name);
+ rc = -EINVAL;
+ }
+ }
+ raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
+ return rc;
+}
+
+static int update_insn_emulation_mode(struct insn_emulation *insn,
+ enum insn_emulation_mode prev)
+{
+ int ret = 0;
+
+ switch (prev) {
+ case INSN_UNDEF: /* Nothing to be done */
+ break;
+ case INSN_EMULATE:
+ remove_emulation_hooks(insn);
+ break;
+ case INSN_HW:
+ if (!run_all_cpu_set_hw_mode(insn, false))
+ pr_notice("Disabled %s support\n", insn->name);
+ break;
+ }
+
+ switch (insn->current_mode) {
+ case INSN_UNDEF:
+ break;
+ case INSN_EMULATE:
+ register_emulation_hooks(insn);
+ break;
+ case INSN_HW:
+ ret = run_all_cpu_set_hw_mode(insn, true);
+ if (!ret)
+ pr_notice("Enabled %s support\n", insn->name);
+ break;
+ }
+
+ return ret;
+}
+
+static void __init register_insn_emulation(struct insn_emulation *insn)
+{
+ unsigned long flags;
+
+ insn->min = INSN_UNDEF;
+
+ switch (insn->status) {
+ case INSN_DEPRECATED:
+ insn->current_mode = INSN_EMULATE;
+ /* Disable the HW mode if it was turned on at early boot time */
+ run_all_cpu_set_hw_mode(insn, false);
+ insn->max = INSN_HW;
+ break;
+ case INSN_OBSOLETE:
+ insn->current_mode = INSN_UNDEF;
+ insn->max = INSN_EMULATE;
+ break;
+ }
+
+ raw_spin_lock_irqsave(&insn_emulation_lock, flags);
+ list_add(&insn->node, &insn_emulation);
+ nr_insn_emulated++;
+ raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
+
+ /* Register any handlers if required */
+ update_insn_emulation_mode(insn, INSN_UNDEF);
+}
+
+static int emulation_proc_handler(struct ctl_table *table, int write,
+ void *buffer, size_t *lenp,
+ loff_t *ppos)
+{
+ int ret = 0;
+ struct insn_emulation *insn = container_of(table->data, struct insn_emulation, current_mode);
+ enum insn_emulation_mode prev_mode = insn->current_mode;
+
+ mutex_lock(&insn_emulation_mutex);
+ ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
+
+ if (ret || !write || prev_mode == insn->current_mode)
+ goto ret;
+
+ ret = update_insn_emulation_mode(insn, prev_mode);
+ if (ret) {
+ /* Mode change failed, revert to previous mode. */
+ insn->current_mode = prev_mode;
+ update_insn_emulation_mode(insn, INSN_UNDEF);
+ }
+ret:
+ mutex_unlock(&insn_emulation_mutex);
+ return ret;
+}
+
+static void __init register_insn_emulation_sysctl(void)
+{
+ unsigned long flags;
+ int i = 0;
+ struct insn_emulation *insn;
+ struct ctl_table *insns_sysctl, *sysctl;
+
+ insns_sysctl = kcalloc(nr_insn_emulated + 1, sizeof(*sysctl),
+ GFP_KERNEL);
+ if (!insns_sysctl)
+ return;
+
+ raw_spin_lock_irqsave(&insn_emulation_lock, flags);
+ list_for_each_entry(insn, &insn_emulation, node) {
+ sysctl = &insns_sysctl[i];
+
+ sysctl->mode = 0644;
+ sysctl->maxlen = sizeof(int);
+
+ sysctl->procname = insn->name;
+ sysctl->data = &insn->current_mode;
+ sysctl->extra1 = &insn->min;
+ sysctl->extra2 = &insn->max;
+ sysctl->proc_handler = emulation_proc_handler;
+ i++;
+ }
+ raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
+
+ register_sysctl("abi", insns_sysctl);
+}
+
/*
* Invoked as core_initcall, which guarantees that the instruction
* emulation is ready for userspace.
--
2.34.1

2023-10-11 10:19:34

by Jinjie Ruan

[permalink] [raw]
Subject: [PATCH v5.15 14/15] arm64: armv8_deprecated: rework deprected instruction handling

From: Mark Rutland <[email protected]>

commit 124c49b1b5d947b7180c5d6cbb09ddf76ea45ea2 upstream.

Support for deprecated instructions can be enabled or disabled at
runtime. To handle this, the code in armv8_deprecated.c registers and
unregisters undef_hooks, and makes cross CPU calls to configure HW
support. This is rather complicated, and the synchronization required to
make this safe ends up serializing the handling of instructions which
have been trapped.

This patch simplifies the deprecated instruction handling by removing
the dynamic registration and unregistration, and changing the trap
handling code to determine whether a handler should be invoked. This
removes the need for dynamic list management, and simplifies the locking
requirements, making it possible to handle trapped instructions entirely
in parallel.

Where changing the emulation state requires a cross-call, this is
serialized by locally disabling interrupts, ensuring that the CPU is not
left in an inconsistent state.

To simplify sysctl management, each insn_emulation is given a separate
sysctl table, permitting these to be registered separately. The core
sysctl code will iterate over all of these when walking sysfs.

I've tested this with userspace programs which use each of the
deprecated instructions, and I've concurrently modified the support
level for each of the features back-and-forth between HW and emulated to
check that there are no spurious SIGILLs sent to userspace when the
support level is changed.

Signed-off-by: Mark Rutland <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: James Morse <[email protected]>
Cc: Joey Gouly <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Will Deacon <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Will Deacon <[email protected]>
Signed-off-by: Jinjie Ruan <[email protected]>
---
arch/arm64/include/asm/traps.h | 19 +-
arch/arm64/kernel/armv8_deprecated.c | 287 ++++++++++++++-------------
arch/arm64/kernel/traps.c | 40 +---
3 files changed, 154 insertions(+), 192 deletions(-)

diff --git a/arch/arm64/include/asm/traps.h b/arch/arm64/include/asm/traps.h
index 6e5826470bea..1f361e2da516 100644
--- a/arch/arm64/include/asm/traps.h
+++ b/arch/arm64/include/asm/traps.h
@@ -13,17 +13,16 @@

struct pt_regs;

-struct undef_hook {
- struct list_head node;
- u32 instr_mask;
- u32 instr_val;
- u64 pstate_mask;
- u64 pstate_val;
- int (*fn)(struct pt_regs *regs, u32 instr);
-};
+#ifdef CONFIG_ARMV8_DEPRECATED
+bool try_emulate_armv8_deprecated(struct pt_regs *regs, u32 insn);
+#else
+static inline bool
+try_emulate_armv8_deprecated(struct pt_regs *regs, u32 insn)
+{
+ return false;
+}
+#endif /* CONFIG_ARMV8_DEPRECATED */

-void register_undef_hook(struct undef_hook *hook);
-void unregister_undef_hook(struct undef_hook *hook);
void force_signal_inject(int signal, int code, unsigned long address, unsigned long err);
void arm64_notify_segfault(unsigned long addr);
void arm64_force_sig_fault(int signo, int code, unsigned long far, const char *str);
diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
index 6555bceff54b..758008020b59 100644
--- a/arch/arm64/kernel/armv8_deprecated.c
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -38,17 +38,24 @@ enum insn_emulation_mode {
enum legacy_insn_status {
INSN_DEPRECATED,
INSN_OBSOLETE,
+ INSN_UNAVAILABLE,
};

struct insn_emulation {
const char *name;
- struct list_head node;
enum legacy_insn_status status;
- struct undef_hook *hooks;
+ bool (*try_emulate)(struct pt_regs *regs,
+ u32 insn);
int (*set_hw_mode)(bool enable);
+
int current_mode;
int min;
int max;
+
+ /*
+ * sysctl for this emulation + a sentinal entry.
+ */
+ struct ctl_table sysctl[2];
};

#define ARM_OPCODE_CONDTEST_FAIL 0
@@ -70,6 +77,7 @@ static unsigned int aarch32_check_condition(u32 opcode, u32 psr)
return ARM_OPCODE_CONDTEST_UNCOND;
}

+#ifdef CONFIG_SWP_EMULATION
/*
* Implement emulation of the SWP/SWPB instructions using load-exclusive and
* store-exclusive.
@@ -228,28 +236,27 @@ static int swp_handler(struct pt_regs *regs, u32 instr)
return 0;
}

-/*
- * Only emulate SWP/SWPB executed in ARM state/User mode.
- * The kernel must be SWP free and SWP{B} does not exist in Thumb.
- */
-static struct undef_hook swp_hooks[] = {
- {
- .instr_mask = 0x0fb00ff0,
- .instr_val = 0x01000090,
- .pstate_mask = PSR_AA32_MODE_MASK,
- .pstate_val = PSR_AA32_MODE_USR,
- .fn = swp_handler
- },
- { }
-};
+static bool try_emulate_swp(struct pt_regs *regs, u32 insn)
+{
+ /* SWP{B} only exists in ARM state and does not exist in Thumb */
+ if (!compat_user_mode(regs) || compat_thumb_mode(regs))
+ return false;
+
+ if ((insn & 0x0fb00ff0) != 0x01000090)
+ return false;
+
+ return swp_handler(regs, insn) == 0;
+}

static struct insn_emulation insn_swp = {
.name = "swp",
.status = INSN_OBSOLETE,
- .hooks = swp_hooks,
+ .try_emulate = try_emulate_swp,
.set_hw_mode = NULL,
};
+#endif /* CONFIG_SWP_EMULATION */

+#ifdef CONFIG_CP15_BARRIER_EMULATION
static int cp15barrier_handler(struct pt_regs *regs, u32 instr)
{
perf_sw_event(PERF_COUNT_SW_EMULATION_FAULTS, 1, regs, regs->pc);
@@ -312,31 +319,29 @@ static int cp15_barrier_set_hw_mode(bool enable)
return 0;
}

-static struct undef_hook cp15_barrier_hooks[] = {
- {
- .instr_mask = 0x0fff0fdf,
- .instr_val = 0x0e070f9a,
- .pstate_mask = PSR_AA32_MODE_MASK,
- .pstate_val = PSR_AA32_MODE_USR,
- .fn = cp15barrier_handler,
- },
- {
- .instr_mask = 0x0fff0fff,
- .instr_val = 0x0e070f95,
- .pstate_mask = PSR_AA32_MODE_MASK,
- .pstate_val = PSR_AA32_MODE_USR,
- .fn = cp15barrier_handler,
- },
- { }
-};
+static bool try_emulate_cp15_barrier(struct pt_regs *regs, u32 insn)
+{
+ if (!compat_user_mode(regs) || compat_thumb_mode(regs))
+ return false;
+
+ if ((insn & 0x0fff0fdf) == 0x0e070f9a)
+ return cp15barrier_handler(regs, insn) == 0;
+
+ if ((insn & 0x0fff0fff) == 0x0e070f95)
+ return cp15barrier_handler(regs, insn) == 0;
+
+ return false;
+}

static struct insn_emulation insn_cp15_barrier = {
.name = "cp15_barrier",
.status = INSN_DEPRECATED,
- .hooks = cp15_barrier_hooks,
+ .try_emulate = try_emulate_cp15_barrier,
.set_hw_mode = cp15_barrier_set_hw_mode,
};
+#endif /* CONFIG_CP15_BARRIER_EMULATION */

+#ifdef CONFIG_SETEND_EMULATION
static int setend_set_hw_mode(bool enable)
{
if (!cpu_supports_mixed_endian_el0())
@@ -384,61 +389,41 @@ static int t16_setend_handler(struct pt_regs *regs, u32 instr)
return rc;
}

-static struct undef_hook setend_hooks[] = {
- {
- .instr_mask = 0xfffffdff,
- .instr_val = 0xf1010000,
- .pstate_mask = PSR_AA32_MODE_MASK,
- .pstate_val = PSR_AA32_MODE_USR,
- .fn = a32_setend_handler,
- },
- {
- /* Thumb mode */
- .instr_mask = 0xfffffff7,
- .instr_val = 0x0000b650,
- .pstate_mask = (PSR_AA32_T_BIT | PSR_AA32_MODE_MASK),
- .pstate_val = (PSR_AA32_T_BIT | PSR_AA32_MODE_USR),
- .fn = t16_setend_handler,
- },
- {}
-};
+static bool try_emulate_setend(struct pt_regs *regs, u32 insn)
+{
+ if (compat_thumb_mode(regs) &&
+ (insn & 0xfffffff7) == 0x0000b650)
+ return t16_setend_handler(regs, insn) == 0;
+
+ if (compat_user_mode(regs) &&
+ (insn & 0xfffffdff) == 0xf1010000)
+ return a32_setend_handler(regs, insn) == 0;
+
+ return false;
+}

static struct insn_emulation insn_setend = {
.name = "setend",
.status = INSN_DEPRECATED,
- .hooks = setend_hooks,
+ .try_emulate = try_emulate_setend,
.set_hw_mode = setend_set_hw_mode,
};
+#endif /* CONFIG_SETEND_EMULATION */
+
+static struct insn_emulation *insn_emulations[] = {
+#ifdef CONFIG_SWP_EMULATION
+ &insn_swp,
+#endif
+#ifdef CONFIG_CP15_BARRIER_EMULATION
+ &insn_cp15_barrier,
+#endif
+#ifdef CONFIG_SETEND_EMULATION
+ &insn_setend,
+#endif
+};

-static LIST_HEAD(insn_emulation);
-static int nr_insn_emulated __initdata;
-static DEFINE_RAW_SPINLOCK(insn_emulation_lock);
static DEFINE_MUTEX(insn_emulation_mutex);

-static void register_emulation_hooks(struct insn_emulation *insn)
-{
- struct undef_hook *hook;
-
- BUG_ON(!insn->hooks);
-
- for (hook = insn->hooks; hook->instr_mask; hook++)
- register_undef_hook(hook);
-
- pr_notice("Registered %s emulation handler\n", insn->name);
-}
-
-static void remove_emulation_hooks(struct insn_emulation *insn)
-{
- struct undef_hook *hook;
-
- BUG_ON(!insn->hooks);
-
- for (hook = insn->hooks; hook->instr_mask; hook++)
- unregister_undef_hook(hook);
-
- pr_notice("Removed %s emulation handler\n", insn->name);
-}
-
static void enable_insn_hw_mode(void *data)
{
struct insn_emulation *insn = (struct insn_emulation *)data;
@@ -473,20 +458,27 @@ static int run_all_cpu_set_hw_mode(struct insn_emulation *insn, bool enable)
*/
static int run_all_insn_set_hw_mode(unsigned int cpu)
{
+ int i;
int rc = 0;
unsigned long flags;
- struct insn_emulation *insn;

- raw_spin_lock_irqsave(&insn_emulation_lock, flags);
- list_for_each_entry(insn, &insn_emulation, node) {
- bool enable = (insn->current_mode == INSN_HW);
+ /*
+ * Disable IRQs to serialize against an IPI from
+ * run_all_cpu_set_hw_mode(), ensuring the HW is programmed to the most
+ * recent enablement state if the two race with one another.
+ */
+ local_irq_save(flags);
+ for (i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
+ struct insn_emulation *insn = insn_emulations[i];
+ bool enable = READ_ONCE(insn->current_mode) == INSN_HW;
if (insn->set_hw_mode && insn->set_hw_mode(enable)) {
pr_warn("CPU[%u] cannot support the emulation of %s",
cpu, insn->name);
rc = -EINVAL;
}
}
- raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
+ local_irq_restore(flags);
+
return rc;
}

@@ -499,7 +491,6 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
case INSN_UNDEF: /* Nothing to be done */
break;
case INSN_EMULATE:
- remove_emulation_hooks(insn);
break;
case INSN_HW:
if (!run_all_cpu_set_hw_mode(insn, false))
@@ -511,7 +502,6 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
case INSN_UNDEF:
break;
case INSN_EMULATE:
- register_emulation_hooks(insn);
break;
case INSN_HW:
ret = run_all_cpu_set_hw_mode(insn, true);
@@ -523,34 +513,6 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
return ret;
}

-static void __init register_insn_emulation(struct insn_emulation *insn)
-{
- unsigned long flags;
-
- insn->min = INSN_UNDEF;
-
- switch (insn->status) {
- case INSN_DEPRECATED:
- insn->current_mode = INSN_EMULATE;
- /* Disable the HW mode if it was turned on at early boot time */
- run_all_cpu_set_hw_mode(insn, false);
- insn->max = INSN_HW;
- break;
- case INSN_OBSOLETE:
- insn->current_mode = INSN_UNDEF;
- insn->max = INSN_EMULATE;
- break;
- }
-
- raw_spin_lock_irqsave(&insn_emulation_lock, flags);
- list_add(&insn->node, &insn_emulation);
- nr_insn_emulated++;
- raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
-
- /* Register any handlers if required */
- update_insn_emulation_mode(insn, INSN_UNDEF);
-}
-
static int emulation_proc_handler(struct ctl_table *table, int write,
void *buffer, size_t *lenp,
loff_t *ppos)
@@ -568,7 +530,7 @@ static int emulation_proc_handler(struct ctl_table *table, int write,
ret = update_insn_emulation_mode(insn, prev_mode);
if (ret) {
/* Mode change failed, revert to previous mode. */
- insn->current_mode = prev_mode;
+ WRITE_ONCE(insn->current_mode, prev_mode);
update_insn_emulation_mode(insn, INSN_UNDEF);
}
ret:
@@ -576,21 +538,34 @@ static int emulation_proc_handler(struct ctl_table *table, int write,
return ret;
}

-static void __init register_insn_emulation_sysctl(void)
+static void __init register_insn_emulation(struct insn_emulation *insn)
{
- unsigned long flags;
- int i = 0;
- struct insn_emulation *insn;
- struct ctl_table *insns_sysctl, *sysctl;
+ struct ctl_table *sysctl;
+
+ insn->min = INSN_UNDEF;
+
+ switch (insn->status) {
+ case INSN_DEPRECATED:
+ insn->current_mode = INSN_EMULATE;
+ /* Disable the HW mode if it was turned on at early boot time */
+ run_all_cpu_set_hw_mode(insn, false);
+ insn->max = INSN_HW;
+ break;
+ case INSN_OBSOLETE:
+ insn->current_mode = INSN_UNDEF;
+ insn->max = INSN_EMULATE;
+ break;
+ case INSN_UNAVAILABLE:
+ insn->current_mode = INSN_UNDEF;
+ insn->max = INSN_UNDEF;
+ break;
+ }

- insns_sysctl = kcalloc(nr_insn_emulated + 1, sizeof(*sysctl),
- GFP_KERNEL);
- if (!insns_sysctl)
- return;
+ /* Program the HW if required */
+ update_insn_emulation_mode(insn, INSN_UNDEF);

- raw_spin_lock_irqsave(&insn_emulation_lock, flags);
- list_for_each_entry(insn, &insn_emulation, node) {
- sysctl = &insns_sysctl[i];
+ if (insn->status != INSN_UNAVAILABLE) {
+ sysctl = &insn->sysctl[0];

sysctl->mode = 0644;
sysctl->maxlen = sizeof(int);
@@ -600,11 +575,34 @@ static void __init register_insn_emulation_sysctl(void)
sysctl->extra1 = &insn->min;
sysctl->extra2 = &insn->max;
sysctl->proc_handler = emulation_proc_handler;
- i++;
+
+ register_sysctl("abi", sysctl);
}
- raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
+}
+
+bool try_emulate_armv8_deprecated(struct pt_regs *regs, u32 insn)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
+ struct insn_emulation *ie = insn_emulations[i];

- register_sysctl("abi", insns_sysctl);
+ if (ie->status == INSN_UNAVAILABLE)
+ continue;
+
+ /*
+ * A trap may race with the mode being changed
+ * INSN_EMULATE<->INSN_HW. Try to emulate the instruction to
+ * avoid a spurious UNDEF.
+ */
+ if (READ_ONCE(ie->current_mode) == INSN_UNDEF)
+ continue;
+
+ if (ie->try_emulate(regs, insn))
+ return true;
+ }
+
+ return false;
}

/*
@@ -613,24 +611,27 @@ static void __init register_insn_emulation_sysctl(void)
*/
static int __init armv8_deprecated_init(void)
{
- if (IS_ENABLED(CONFIG_SWP_EMULATION))
- register_insn_emulation(&insn_swp);
+ int i;

- if (IS_ENABLED(CONFIG_CP15_BARRIER_EMULATION))
- register_insn_emulation(&insn_cp15_barrier);
+#ifdef CONFIG_SETEND_EMULATION
+ if (!system_supports_mixed_endian_el0()) {
+ insn_setend.status = INSN_UNAVAILABLE;
+ pr_info("setend instruction emulation is not supported on this system\n");
+ }

- if (IS_ENABLED(CONFIG_SETEND_EMULATION)) {
- if (system_supports_mixed_endian_el0())
- register_insn_emulation(&insn_setend);
- else
- pr_info("setend instruction emulation is not supported on this system\n");
+#endif
+ for (i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
+ struct insn_emulation *ie = insn_emulations[i];
+
+ if (ie->status == INSN_UNAVAILABLE)
+ continue;
+
+ register_insn_emulation(ie);
}

cpuhp_setup_state_nocalls(CPUHP_AP_ARM64_ISNDEP_STARTING,
"arm64/isndep:starting",
run_all_insn_set_hw_mode, NULL);
- register_insn_emulation_sysctl();
-
return 0;
}

diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 1f9285a35560..c71074cb2bef 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -373,27 +373,6 @@ void arm64_skip_faulting_instruction(struct pt_regs *regs, unsigned long size)
regs->pstate &= ~PSR_BTYPE_MASK;
}

-static LIST_HEAD(undef_hook);
-static DEFINE_RAW_SPINLOCK(undef_lock);
-
-void register_undef_hook(struct undef_hook *hook)
-{
- unsigned long flags;
-
- raw_spin_lock_irqsave(&undef_lock, flags);
- list_add(&hook->node, &undef_hook);
- raw_spin_unlock_irqrestore(&undef_lock, flags);
-}
-
-void unregister_undef_hook(struct undef_hook *hook)
-{
- unsigned long flags;
-
- raw_spin_lock_irqsave(&undef_lock, flags);
- list_del(&hook->node);
- raw_spin_unlock_irqrestore(&undef_lock, flags);
-}
-
static int user_insn_read(struct pt_regs *regs, u32 *insnp)
{
u32 instr;
@@ -425,23 +404,6 @@ static int user_insn_read(struct pt_regs *regs, u32 *insnp)
return 0;
}

-static int call_undef_hook(struct pt_regs *regs, u32 instr)
-{
- struct undef_hook *hook;
- unsigned long flags;
- int (*fn)(struct pt_regs *regs, u32 instr) = NULL;
-
- raw_spin_lock_irqsave(&undef_lock, flags);
- list_for_each_entry(hook, &undef_hook, node)
- if ((instr & hook->instr_mask) == hook->instr_val &&
- (regs->pstate & hook->pstate_mask) == hook->pstate_val)
- fn = hook->fn;
-
- raw_spin_unlock_irqrestore(&undef_lock, flags);
-
- return fn ? fn(regs, instr) : 1;
-}
-
void force_signal_inject(int signal, int code, unsigned long address, unsigned long err)
{
const char *desc;
@@ -502,7 +464,7 @@ void do_el0_undef(struct pt_regs *regs, unsigned long esr)
if (try_emulate_mrs(regs, insn))
return;

- if (call_undef_hook(regs, insn) == 0)
+ if (try_emulate_armv8_deprecated(regs, insn))
return;

out_err:
--
2.34.1

2023-10-16 08:06:16

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v5.15 00/15] arm64: Fix a concurrency issue in emulation_proc_handler()

On Wed, Oct 11, 2023 at 10:06:40AM +0000, Jinjie Ruan wrote:
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> In linux-6.1, the related code is refactored in commit 124c49b1b5d9
> ("arm64: armv8_deprecated: rework deprected instruction handling") and this
> issue was incidentally fixed. This patch set try to adapt the refactoring
> patches to stable 5.15 to solve the problem of repeated addition of linked
> lists described below.
>
> How to reproduce:
> CONFIG_ARMV8_DEPRECATED=y, CONFIG_SWP_EMULATION=y, and CONFIG_DEBUG_LIST=y,
> then launch two shell executions:
> #!/bin/bash
> while [ 1 ];
> do
> echo 1 > /proc/sys/abi/swp
> done
>
> or "echo 1 > /proc/sys/abi/swp" and then launch two shell executions:
> #!/bin/bash
> while [ 1 ];
> do
> echo 0 > /proc/sys/abi/swp
> done
>
> In emulation_proc_handler(), read and write operations are performed on
> insn->current_mode. In the concurrency scenario, mutex only protects
> writing insn->current_mode, and not protects the read. Suppose there are
> two concurrent tasks, task1 updates insn->current_mode to INSN_EMULATE
> in the critical section, the prev_mode of task2 is still the old data
> INSN_UNDEF of insn->current_mode. As a result, two tasks call
> update_insn_emulation_mode twice with prev_mode = INSN_UNDEF and
> current_mode = INSN_EMULATE, then call register_emulation_hooks twice,
> resulting in a list_add double problem.
>
> commit 124c49b1b5d9 ("arm64: armv8_deprecated: rework deprected instruction
> handling") remove the dynamic registration and unregistration so remove the
> register_undef_hook() function, so the below problem was incidentally
> fixed.
>
> Call trace:
> __list_add_valid+0xd8/0xe4
> register_undef_hook+0x94/0x13c
> update_insn_emulation_mode+0xd0/0x12c
> emulation_proc_handler+0xd8/0xf4
> proc_sys_call_handler+0x140/0x250
> proc_sys_write+0x1c/0x2c
> new_sync_write+0xec/0x18c
> vfs_write+0x214/0x2ac
> ksys_write+0x70/0xfc
> __arm64_sys_write+0x24/0x30
> el0_svc_common.constprop.0+0x7c/0x1bc
> do_el0_svc+0x2c/0x94
> el0_svc+0x20/0x30
> el0_sync_handler+0xb0/0xb4
> el0_sync+0x160/0x180
>
> Call trace:
> __list_del_entry_valid+0xac/0x110
> unregister_undef_hook+0x34/0x80
> update_insn_emulation_mode+0xf0/0x180
> emulation_proc_handler+0x8c/0xd8
> proc_sys_call_handler+0x1d8/0x208
> proc_sys_write+0x14/0x20
> new_sync_write+0xf0/0x190
> vfs_write+0x304/0x388
> ksys_write+0x6c/0x100
> __arm64_sys_write+0x1c/0x28
> el0_svc_common.constprop.4+0x68/0x188
> do_el0_svc+0x24/0xa0
> el0_svc+0x14/0x20
> el0_sync_handler+0x90/0xb8
> el0_sync+0x160/0x180
>
> The first 5 patches is a patch set which provides context for subsequent
> refactoring 9 patches, especially commit 0f2cb928a154 ("arm64:
> consistently pass ESR_ELx to die()") which modify do_undefinstr() to add a
> ESR_ELx value arg, and then commit 61d64a376ea8 ("arm64: split EL0/EL1
> UNDEF handlers") splits do_undefinstr() handler into separate
> do_el0_undef() and do_el1_undef() handlers.
>
> The 9 patches after that is another refactoring patch set, which is in
> preparation for the main rework commit 124c49b1b5d9 ("arm64:
> armv8_deprecated: rework deprected instruction handling"). To remove struct
> undef_hook, commit bff8f413c71f ("arm64: factor out EL1 SSBS emulation
> hook") factor out EL1 SSBS emulation hook, which also avoid call
> call_undef_hook() in do_el1_undef(), commit f5962add74b6 ("arm64: rework
> EL0 MRS emulation") factor out EL0 MRS emulation hook, which also prepare
> for replacing call_undef_hook() in do_el0_undef(). To replace
> call_undef_hook() function, commit 61d64a376ea8 ("arm64: split EL0/EL1
> UNDEF handlers") split the do_undefinstr() into do_el0_undef() and
> do_el1_undef() functions, and commit dbfbd87efa79 ("arm64: factor insn
> read out of call_undef_hook()") factor user_insn_read() from
> call_undef_hook() so the main rework patch can replace the
> call_undef_hook() in do_el0_undef().
>
> The last patch is a bugfix for the main rework patch.
>
> I've tested this with userspace programs which use each of the
> deprecated instructions on Raspberry Pi 4B KVM/Qemu, and I've concurrently
> modified the support level for each of the features back-and-forth between
> HW and emulated to check that there are no oops or above repeated addition
> or deletion call trace.
>
> Fixes: af483947d472 ("arm64: fix oops in concurrently setting insn_emulation sysctls")
> Cc: [email protected]#5.15.x
> Cc: [email protected]
> Signed-off-by: Jinjie Ruan <[email protected]>

All now queued up, thanks.

greg k-h