Message ID | 20230131065211.2826133-1-ruanjinjie@huawei.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp2597449wrn; Mon, 30 Jan 2023 22:57:06 -0800 (PST) X-Google-Smtp-Source: AK7set+LzDxU60E1A8+Cttok6R8mXLy9LdG6QH4tkuEYGctQmxdrHEncvZTLS2FuG2pqkHRohppj X-Received: by 2002:a05:6a00:1d9b:b0:593:c7d4:22b0 with SMTP id z27-20020a056a001d9b00b00593c7d422b0mr6054812pfw.2.1675148225737; Mon, 30 Jan 2023 22:57:05 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1675148225; cv=none; d=google.com; s=arc-20160816; b=nkjinl3MAIK5OqC4h8MyL66+MAb07ZRyc0fYnWjG/DaHcqhK4fcz7CvSJ+iLyR1lN4 y4qUv2fbjr08o7/Fd0K1z6yk81i6EShg8GdhpHqvTl0Z2Z7tc0wB1uE640Uqkt6/FBJN o3To/UOLQKLIvWRnNrjysrwytXVjOqflNQs19GEK8qURLUG40gtTricGJOxEtkWuAUHs vrtaVe2Net8Evt6zTiRw7ootxx+SWcLFOMCNAt4V2pnlLTKVMVXilRapyBK1tLDCgjDj z5kqSFH8nwY9Q54QbJwyJBPVm72grNO+Rs6VlNgmg9AQdU10apJFasOZ+qhK9tqmwRU2 UfPg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from; bh=+yWvEaAasrIE4RPxMZl5qyilvZadxBv6cPltglDCMxQ=; b=UGxpAI0q8DjPoq6MMNs1qbpJp6h34BWmTiA0URJ1muJqhQU2zdk0ogsUnH+lNGZeYg uzYaG7K8xDQWC4VqBpcFhQMwYBTzu4CDSrjlgG/AWbsmHZ09v8YvBJ7/+ubiAOm91y0B QML4tEB4Bq7WCUI/zYOFi3cJDEnGg3ENF82SxSoD54eXZhDfvC1tLEK/1sMMtLei33yz A49fv9VmJ7dD5pk/aNjmBhrDNUQGKc4DR8kYt1tbENYKj5h/P9e+JKOYqlg7DFX088Cr KCosuPTHpx89j60U6Cn1PqREIMsCTkuMDMmtAUrfDLsUzw7oi/yfrsvh6ic7RYDKVHfc GGPQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=huawei.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id w66-20020a623045000000b0059338de4684si11196328pfw.255.2023.01.30.22.56.53; Mon, 30 Jan 2023 22:57:05 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=huawei.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230149AbjAaGyl (ORCPT <rfc822;maxin.john@gmail.com> + 99 others); Tue, 31 Jan 2023 01:54:41 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49964 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229559AbjAaGyi (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 31 Jan 2023 01:54:38 -0500 Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 908F524118 for <linux-kernel@vger.kernel.org>; Mon, 30 Jan 2023 22:54:37 -0800 (PST) Received: from kwepemi500008.china.huawei.com (unknown [172.30.72.54]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4P5bND3SnzzJrW6; Tue, 31 Jan 2023 14:53:00 +0800 (CST) Received: from huawei.com (10.67.175.83) by kwepemi500008.china.huawei.com (7.221.188.139) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.34; Tue, 31 Jan 2023 14:54:34 +0800 From: ruanjinjie <ruanjinjie@huawei.com> To: <catalin.marinas@arm.com>, <will@kernel.org>, <haibinzhang@tencent.com>, <hewenliang4@huawei.com>, <linux-arm-kernel@lists.infradead.org>, <linux-kernel@vger.kernel.org>, <gregkh@linuxfoundation.org> CC: <ruanjinjie@huawei.com> Subject: [PATCH v5.10] arm64: fix a concurrency issue in emulation_proc_handler() Date: Tue, 31 Jan 2023 14:52:11 +0800 Message-ID: <20230131065211.2826133-1-ruanjinjie@huawei.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Content-Transfer-Encoding: 7BIT Content-Type: text/plain; charset=US-ASCII X-Originating-IP: [10.67.175.83] X-ClientProxiedBy: dggems706-chm.china.huawei.com (10.3.19.183) To kwepemi500008.china.huawei.com (7.221.188.139) X-CFilter-Loop: Reflected X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_MED, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1756520226092674863?= X-GMAIL-MSGID: =?utf-8?q?1756520226092674863?= |
Series |
[v5.10] arm64: fix a concurrency issue in emulation_proc_handler()
|
|
Commit Message
Jinjie Ruan
Jan. 31, 2023, 6:52 a.m. UTC
This patch is addressing an issue in stable linux-5.10 only. In linux-6.1, the related code is refactored in 124c49b1b("arm64: armv8_deprecated: rework deprected instruction handling") and this issue was incidentally fixed. However, the patch changes a lot and is not specific to this issue. This patch is designed to solve the problem of repeated addition of linked lists described below with few changes. 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. 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 Fixes: af483947d472 ("arm64: fix oops in concurrently setting insn_emulation sysctls") Cc: stable@vger.kernel.org#5.10.x Cc: gregkh@linuxfoundation.org Signed-off-by: ruanjinjie <ruanjinjie@huawei.com> Acked-by: Mark Rutland <mark.rutland@arm.com> --- arch/arm64/kernel/armv8_deprecated.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
Comments
On Tue, Jan 31, 2023 at 02:52:11PM +0800, ruanjinjie wrote: > This patch is addressing an issue in stable linux-5.10 only. > > In linux-6.1, the related code is refactored in 124c49b1b("arm64: > armv8_deprecated: rework deprected instruction handling") and > this issue was incidentally fixed. However, the patch changes a lot and > is not specific to this issue. Then what about 5.15.y? You can not upgrade to that kernel and have a regression, right? And nit, you need a ' ' before the '(' character. But why can we just not take the original commit that fixed this issue? That way almost always is the best (prevents regressions, makes backports easier, is actually tested, etc.) ? thanks, greg k-h
On 2023/1/31 15:27, Greg KH wrote: > On Tue, Jan 31, 2023 at 02:52:11PM +0800, ruanjinjie wrote: >> This patch is addressing an issue in stable linux-5.10 only. >> >> In linux-6.1, the related code is refactored in 124c49b1b("arm64: >> armv8_deprecated: rework deprected instruction handling") and >> this issue was incidentally fixed. However, the patch changes a lot and >> is not specific to this issue. > > Then what about 5.15.y? You can not upgrade to that kernel and have a > regression, right? This patch has a pre-dependency af483947d ("arm64: fix oops in concurrently setting insn_emulation sysctls"), which has not merged into branches except 5.10.y, so the other branches don't apply. > > And nit, you need a ' ' before the '(' character. > > But why can we just not take the original commit that fixed this issue? > That way almost always is the best (prevents regressions, makes > backports easier, is actually tested, etc.) ? Thank you! It is ok to take the original commit to fix this issue. > > thanks, > > greg k-h
On Tue, Jan 31, 2023 at 04:23:19PM +0800, Ruan Jinjie wrote: > > > On 2023/1/31 15:27, Greg KH wrote: > > On Tue, Jan 31, 2023 at 02:52:11PM +0800, ruanjinjie wrote: > >> This patch is addressing an issue in stable linux-5.10 only. > >> > >> In linux-6.1, the related code is refactored in 124c49b1b("arm64: > >> armv8_deprecated: rework deprected instruction handling") and > >> this issue was incidentally fixed. However, the patch changes a lot and > >> is not specific to this issue. > > > > Then what about 5.15.y? You can not upgrade to that kernel and have a > > regression, right? > > This patch has a pre-dependency af483947d ("arm64: fix oops in > concurrently setting insn_emulation sysctls"), which has not merged into > branches except 5.10.y, so the other branches don't apply. So there is no bug in 5.15.y? That's confusing. > > And nit, you need a ' ' before the '(' character. > > > > But why can we just not take the original commit that fixed this issue? > > That way almost always is the best (prevents regressions, makes > > backports easier, is actually tested, etc.) ? > > Thank you! It is ok to take the original commit to fix this issue. Please submit it after testing. thanks, greg k-h
diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c index 91b8a8378ba3..5f8da0d7b832 100644 --- a/arch/arm64/kernel/armv8_deprecated.c +++ b/arch/arm64/kernel/armv8_deprecated.c @@ -208,10 +208,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)