Messages in this thread Patch in this message | | | Date | Fri, 1 Jul 2022 12:17:02 +0100 | From | Will Deacon <> | Subject | Re: [PATCH] arm64: fix oops in concurrently setting insn_emulation sysctls |
| |
On Fri, Jul 01, 2022 at 02:27:17AM +0000, 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, Add mutal exclusion covering related code section.
typo: mutual
> Signed-off-by: Haibin Zhang <haibinzhang@tencent.com> > --- > arch/arm64/kernel/armv8_deprecated.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c > index 6875a16..c519792 100644 > --- a/arch/arm64/kernel/armv8_deprecated.c > +++ b/arch/arm64/kernel/armv8_deprecated.c > @@ -207,8 +207,12 @@ 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; > - enum insn_emulation_mode prev_mode = insn->current_mode; > + struct insn_emulation *insn; > + enum insn_emulation_mode prev_mode; > + > + raw_spin_lock(&insn_emulation_lock); > + insn = (struct insn_emulation *) table->data; > + prev_mode = insn->current_mode; > > table->data = &insn->current_mode; > ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos); > @@ -224,6 +228,7 @@ static int emulation_proc_handler(struct ctl_table *table, int write, > } > ret: > table->data = insn; > + raw_spin_unlock(&insn_emulation_lock); > return ret;
This looks very similar to the patch previously posted here:
https://lore.kernel.org/r/20220128090324.2727688-1-hewenliang4@huawei.com
but Catalin's suggestion was ignored:
https://lore.kernel.org/all/Yf0dxon1d07rzxZH@arm.com/
Please can have you send a v2 along the line that he suggested?
I also think a mutex is probably better than a spinlock given that we can end up cross-calling in the proc handler. Thanks,
Will
| |