2023-10-30 06:38:20

by Jinjie Ruan

[permalink] [raw]
Subject: [PATCH v4.19] arm64: fix a concurrency issue in emulation_proc_handler()

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: [email protected]#4.19.x
Cc: [email protected]
Signed-off-by: Jinjie Ruan <[email protected]>
Acked-by: Mark Rutland <[email protected]>
---
arch/arm64/kernel/armv8_deprecated.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

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)
--
2.34.1


2023-10-31 11:18:36

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v4.19] arm64: fix a concurrency issue in emulation_proc_handler()

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

2023-10-31 11:53:02

by Jinjie Ruan

[permalink] [raw]
Subject: Re: [PATCH v4.19] arm64: fix a concurrency issue in emulation_proc_handler()



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=archiver.kernel.org@lists.infradead.org>
> 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 <[email protected]>; 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 [email protected];
> 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 <[email protected]>
> To: Jinjie Ruan <[email protected]>
> Cc: [email protected], [email protected], [email protected],
> [email protected], [email protected]
> Subject: Re: [PATCH v4.19] arm64: fix a concurrency issue in
> emulation_proc_handler()
> Message-ID: <2023103103-revision-gigantic-cc05@gregkh>
> References: <[email protected]>
> MIME-Version: 1.0
> Content-Disposition: inline
> In-Reply-To: <[email protected]>
> 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: [email protected]
> 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:[email protected]?subject=unsubscribe>
> List-Archive: <http://lists.infradead.org/pipermail/linux-arm-kernel/>
> List-Post: <mailto:[email protected]>
> List-Help: <mailto:[email protected]?subject=help>
> List-Subscribe: <http://lists.infradead.org/mailman/listinfo/linux-arm-kernel>,
> <mailto:[email protected]?subject=subscribe>
> Content-Type: text/plain; charset="us-ascii"
> Content-Transfer-Encoding: 7bit
> Sender: "linux-arm-kernel" <[email protected]>
> Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org
>
> 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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel