lkml.org 
[lkml]   [2022]   [Jul]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH V2] arm64: fix oops in concurrently setting insn_emulation sysctls
On Sat, 02 Jul 2022, haibinzhang(张海斌) wrote:

> How to reproduce:
> launch two shell executions:
> #!/bin/bash
> while [ 1 ];
> do
> echo 1 > /proc/sys/abi/swp
> done
>
> Oops info:
> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000010
> Internal error: Oops: 96000006 [#1] SMP
> Call trace:
> update_insn_emulation_mode+0xc0/0x148
> emulation_proc_handler+0x64/0xb8
> proc_sys_call_handler+0x9c/0xf8
> proc_sys_write+0x18/0x20
> __vfs_write+0x20/0x48
> vfs_write+0xe4/0x1d0
> ksys_write+0x70/0xf8
> __arm64_sys_write+0x20/0x28
> el0_svc_common.constprop.0+0x7c/0x1c0
> el0_svc_handler+0x2c/0xa0
> el0_svc+0x8/0x200
>
> emulation_proc_handler changes table->data for proc_dointvec_minmax
> and so it isn't allowed to reenter before restoring table->data,
> which isn't right now.
> To fix this issue, keep the table->data as &insn->current_mode and
> use container_of() to retrieve the insn pointer. Another mutex is
> used to protect against the current_mode update but not for retrieving
> insn_emulation as table->data is no longer changing.

Looks as though this lost its Fixes tag during the rework.

Fixes: 587064b610c7 ("arm64: Add framework for legacy instruction emulation")

Will, are you able to add this retroactively?

> Signed-off-by: hewenliang <hewenliang4@huawei.com>
> Signed-off-by: Haibin Zhang <haibinzhang@tencent.com>
> ---
> arch/arm64/kernel/armv8_deprecated.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
> index 6875a16b09d2..fb0e7c7b2e20 100644
> --- a/arch/arm64/kernel/armv8_deprecated.c
> +++ b/arch/arm64/kernel/armv8_deprecated.c
> @@ -59,6 +59,7 @@ struct insn_emulation {
> 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_ops *ops)
> {
> @@ -207,10 +208,10 @@ static int emulation_proc_handler(struct ctl_table *table, int write,
> loff_t *ppos)
> {
> int ret = 0;
> - struct insn_emulation *insn = (struct insn_emulation *) table->data;
> + struct insn_emulation *insn = container_of(table->data, struct insn_emulation, current_mode);
> enum insn_emulation_mode prev_mode = insn->current_mode;
>
> - table->data = &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)
> @@ -223,7 +224,7 @@ static int emulation_proc_handler(struct ctl_table *table, int write,
> update_insn_emulation_mode(insn, INSN_UNDEF);
> }
> ret:
> - table->data = insn;
> + mutex_unlock(&insn_emulation_mutex);
> return ret;
> }
>
> @@ -247,7 +248,7 @@ static void __init register_insn_emulation_sysctl(void)
> sysctl->maxlen = sizeof(int);
>
> sysctl->procname = insn->ops->name;
> - sysctl->data = insn;
> + sysctl->data = &insn->current_mode;
> sysctl->extra1 = &insn->min;
> sysctl->extra2 = &insn->max;
> sysctl->proc_handler = emulation_proc_handler;
> --
> 2.34.1
>

--
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

\
 
 \ /
  Last update: 2022-07-20 14:24    [W:0.709 / U:0.524 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site