[v4.19] arm64: fix a concurrency issue in emulation_proc_handler()

Commit Message

Jinjie Ruan Oct. 30, 2023, 6:37 a.m. UTC

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. I have adapted the patch set to linux stable
5.10. However, 4.19 and 5.10 are too different and the patch set is
hard to adapt to 4.19.

This patch is to solve the problem of repeated addition of linked lists
described below with few changes.

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 aunch 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.

After applying this patch, the following list add or list del double
warnings never occur.

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

Fixes: af483947d472 ("arm64: fix oops in concurrently setting insn_emulation sysctls")
Cc: [e-mail]#4.19.x
Cc: [e-mail]
Signed-off-by: Jinjie Ruan <[e-mail]>
Acked-by: Mark Rutland <[e-mail]>
---
 arch/arm64/kernel/armv8_deprecated.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Greg Kroah-Hartman Oct. 31, 2023, 11:18 a.m. UTC | #1

On Mon, Oct 30, 2023 at 06:37:09AM +0000, Jinjie Ruan wrote:
> 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. I have adapted the patch set to linux stable
> 5.10. However, 4.19 and 5.10 are too different and the patch set is
> hard to adapt to 4.19.

This is also needed for 5.4.y, right?  Now queued up for both.

thanks,

greg k-h

Jinjie Ruan Oct. 31, 2023, 11:52 a.m. UTC | #2

On 2023/10/31 19:18, Greg KH wrote:
> On Mon, Oct 30, 2023 at 06:37:09AM +0000, Jinjie Ruan wrote:
>> 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. I have adapted the patch set to linux stable
>> 5.10. However, 4.19 and 5.10 are too different and the patch set is
>> hard to adapt to 4.19.
> 
> This is also needed for 5.4.y, right?  Now queued up for both.

Right! 5.4.y also need it. Thank you very much.

> 
> thanks,
> 
> greg k-h
> 
> From mboxrd@z Thu Jan  1 00:00:00 1970
> Return-Path: <linux-arm-kernel-bounces+linux-arm-kernel=[e-mail]>
> X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on
> 	aws-us-west-2-korg-lkml-1.web.codeaurora.org
> Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133])
> 	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
> 	(No client certificate requested)
> 	by smtp.lore.kernel.org (Postfix) with ESMTPS id 81AC2C4332F
> 	for <[e-mail]>; Tue, 31 Oct 2023 11:19:01 +0000 (UTC)
> DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;
> 	d=lists.infradead.org; s=bombadil.20210309; h=Sender:
> 	Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post:
> 	List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:
> 	Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:
> 	Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:
> 	List-Owner; bh=sHNb70g1HDu0TF54RigSJfYMEz5vBtgGnOr5JOh2HyQ=; b=iFgVKiA+IkzhII
> 	m4gw5nvcyNNhqjko8wsNMeG+gPyXUAkeZPfjW2w2erf+P6b4YZFQ0iuIwjDLzv/SBKpYa+vWmbv5E
> 	/TisXFYw7ISsR7nmNdQMmZWs5VVeGYO7MlP1u1/2Cw7EfSZ28ynjndjexd7fVHeKdB5R2ciVsWPVk
> 	n2v29BlvlRs7MfzHJ2JKYyd0JqFoGKTDPxQA5G2l3+aKt1p+5Ujqvv+adKlaU9ZO+qd/DErfcwATR
> 	BGvjbVo1uYIXyMXqoODe/dZW7BTcnC9WQb5cxnq3GUP2AzfEaStJEYNvX9e6DQKM89EfhU1EBLZMt
> 	e7MdaC4WSAweX4R63wQA==;
> Received: from localhost ([::1] helo=bombadil.infradead.org)
> 	by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux))
> 	id 1qxmlU-0059OC-1J;
> 	Tue, 31 Oct 2023 11:18:32 +0000
> Received: from ams.source.kernel.org ([145.40.68.75])
> 	by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux))
> 	id 1qxmlR-0059Ma-34
> 	for [e-mail];
> 	Tue, 31 Oct 2023 11:18:31 +0000
> Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58])
> 	by ams.source.kernel.org (Postfix) with ESMTP id 8E876B810AC;
> 	Tue, 31 Oct 2023 11:18:21 +0000 (UTC)
> Received: by smtp.kernel.org (Postfix) with ESMTPSA id 5E680C433C8;
> 	Tue, 31 Oct 2023 11:18:20 +0000 (UTC)
> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org;
> 	s=korg; t=1698751100;
> 	bh=FrkgzgA5lVgOv9L7nzUu3Pnkn7b1h5ivphcK2rmy0/k=;
> 	h=Date:From:To:Cc:Subject:References:In-Reply-To:From;
> 	b=0SnFDTNYaKYCbdOzEos6VTDFSfwrsP68TOPQnm1RW6NlEHxCMLjW5JSmUoK5ya9Yn
> 	 wvsTROrz11Ki59JxvLNnYJdmRMpdVzi6WC2gjr3O2xq6Rd0NE5r/0OMIHmdM+ucQvJ
> 	 c0k+YQyEE43MNueByonhQ6Zw4tfdwPoxruNsrMr4=
> Date: Tue, 31 Oct 2023 12:18:17 +0100
> From: Greg KH <[e-mail]>
> To: Jinjie Ruan <[e-mail]>
> Cc: [e-mail], [e-mail], [e-mail],
> 	[e-mail], [e-mail]
> Subject: Re: [PATCH v4.19] arm64: fix a concurrency issue in
>  emulation_proc_handler()
> Message-ID: <2023103103-revision-gigantic-cc05@gregkh>
> References: <[e-mail]>
> MIME-Version: 1.0
> Content-Disposition: inline
> In-Reply-To: <[e-mail]>
> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 
> X-CRM114-CacheID: sfid-20231031_041830_159334_D1F4D83C 
> X-CRM114-Status: GOOD (  10.35  )
> X-BeenThere: [e-mail]
> X-Mailman-Version: 2.1.34
> Precedence: list
> List-Id: <linux-arm-kernel.lists.infradead.org>
> List-Unsubscribe: <http://lists.infradead.org/mailman/options/linux-arm-kernel>,
>  <mailto:[e-mail]?subject=unsubscribe>
> List-Archive: <http://lists.infradead.org/pipermail/linux-arm-kernel/>
> List-Post: <mailto:[e-mail]>
> List-Help: <mailto:[e-mail]?subject=help>
> List-Subscribe: <http://lists.infradead.org/mailman/listinfo/linux-arm-kernel>,
>  <mailto:[e-mail]?subject=subscribe>
> Content-Type: text/plain; charset="us-ascii"
> Content-Transfer-Encoding: 7bit
> Sender: "linux-arm-kernel" <[e-mail]>
> Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=[e-mail]
> 
> On Mon, Oct 30, 2023 at 06:37:09AM +0000, Jinjie Ruan wrote:
>> 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. I have adapted the patch set to linux stable
>> 5.10. However, 4.19 and 5.10 are too different and the patch set is
>> hard to adapt to 4.19.
> 
> This is also needed for 5.4.y, right?  Now queued up for both.
> 
> thanks,
> 
> greg k-h
> 
> _______________________________________________
> linux-arm-kernel mailing list
> [e-mail]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Patch

diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
index 7c69a203cdf8..b8d481c3e26d 100644
--- a/arch/arm64/kernel/armv8_deprecated.c
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -211,10 +211,12 @@  static int emulation_proc_handler(struct ctl_table *table, int write,
 				  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;
+	struct insn_emulation *insn;
+	enum insn_emulation_mode prev_mode;
 
 	mutex_lock(&insn_emulation_mutex);
+	insn = container_of(table->data, struct insn_emulation, current_mode);
+	prev_mode = insn->current_mode;
 	ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
 
 	if (ret || !write || prev_mode == insn->current_mode)