From patchwork Tue Aug 22 01:09:34 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Uday Shankar X-Patchwork-Id: 13360026 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 614B9EE49AA for ; Tue, 22 Aug 2023 01:10:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231947AbjHVBKG (ORCPT ); Mon, 21 Aug 2023 21:10:06 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57160 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229449AbjHVBKE (ORCPT ); Mon, 21 Aug 2023 21:10:04 -0400 Received: from mail-pl1-x662.google.com (mail-pl1-x662.google.com [IPv6:2607:f8b0:4864:20::662]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1381213D for ; Mon, 21 Aug 2023 18:10:02 -0700 (PDT) Received: by mail-pl1-x662.google.com with SMTP id d9443c01a7336-1bc0d39b52cso24774275ad.2 for ; Mon, 21 Aug 2023 18:10:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=purestorage.com; s=google2022; t=1692666601; x=1693271401; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=MWqEbU+U7DESXzuQI5TQ4rpqUMVzrWVa2QLuXHSidTE=; b=PKxZ8BarogkkIrj8fKWhjSByzLKF2O+SGPHM9hkCLahYbt0q9vVurHFq7vTCI+Rdzy veM7ysPnKZDrOmRLmLXopv/yKnipxaWdSg4z4VWN2BXr4ObFw5z73qKoEXqtsQg76XXH cjV4QBxxl5sKwToPA6oS3ZjKF3tN0Voe2ud1oL2EFUPn3tBB8TqEJ5GMOupw0g0Rcr6Q OvUYLKKmB9iB606obZfaPf5k4r+Mie3WVOqOfN93bKjs+GlYQRaqSvju/+O6IGZJ6e1t q9LW0QshAJP67NWUTlS2QT5thfSleanZThxHUpRmb7NMW2zFpQczvq5JgezZLYVdRpNK U5JQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1692666601; x=1693271401; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=MWqEbU+U7DESXzuQI5TQ4rpqUMVzrWVa2QLuXHSidTE=; b=Zx5WTiq5RvikG+8bUsfE1+CQIz9DdlHFyaUKDJl8P/ny3X0FUedQrorAXSq1xo1JQZ 4EO0jVHssiAGsU9qqnAYIiH+JZ759n1ZbYkPS4Ips0uA7kWNFNLG1IAjiFk1ElMfJ2Xy LLXhIwjzw7+LBGW5DkkbXEmPX2zcWxDc+0Lf+/BrOnoBb7M4+Pltfp1Dh8srthkRRAVC wdK6iexZB3xwqyOUe1/0/M5HqVcK2HdZTLbVUk0NORcN4MSC778BFq702cHo5ET+Cbie dESM/qNWwRAaiqi1FNwEzniHOUzqeRWT098KnHwo5FdlfnEwUnnh+VEtIFwtzQJOLlva Qbpw== X-Gm-Message-State: AOJu0Yxawmee0xQyOd8TKN4mRAH8MzstvFjedBECPoine3rhci3veuXT jhXriCpqHqjvZWCOoCBH+1f6KbWL/Z4kZUZhC38RvXS0dMz1Kak2XrWPMLM+YJ1AQQ== X-Google-Smtp-Source: AGHT+IHW4XOke+1mTx+7ARgRUCumiY4+/dez95t5GtFyLTloyaKSqErMBzfr7QL6pGyIOb0Uklj3yYadJGUY X-Received: by 2002:a17:902:e888:b0:1bd:edac:af44 with SMTP id w8-20020a170902e88800b001bdedacaf44mr6919811plg.51.1692666601435; Mon, 21 Aug 2023 18:10:01 -0700 (PDT) Received: from c7-smtp.dev.purestorage.com ([2620:125:9007:320:7:32:106:0]) by smtp-relay.gmail.com with ESMTPS id g8-20020a170902740800b001bbb8550252sm327321pll.65.2023.08.21.18.10.01 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 21 Aug 2023 18:10:01 -0700 (PDT) X-Relaying-Domain: purestorage.com Received: from dev-ushankar.dev.purestorage.com (dev-ushankar.dev.purestorage.com [10.7.70.36]) by c7-smtp.dev.purestorage.com (Postfix) with ESMTP id 7C111220D6; Mon, 21 Aug 2023 19:10:00 -0600 (MDT) Received: by dev-ushankar.dev.purestorage.com (Postfix, from userid 1557716368) id 7534AE4332A; Mon, 21 Aug 2023 19:10:00 -0600 (MDT) From: Uday Shankar To: Joern Engel , Kees Cook , Tony Luck , "Guilherme G. Piccoli" , "Rafael J. Wysocki" , Len Brown , James Morse , Borislav Petkov , Huang Ying , Len Brown Cc: Uday Shankar , linux-hardening@vger.kernel.org, linux-acpi@vger.kernel.org Subject: [PATCH] Revert "ACPI, APEI, use raw spinlock in ERST" Date: Mon, 21 Aug 2023 19:09:34 -0600 Message-Id: <20230822010933.2247671-1-ushankar@purestorage.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-hardening@vger.kernel.org The commit commit 3b38bb5f7f06356a89bb2cbf92ad346025e95192 Author: Huang Ying Date: Thu Dec 2 10:40:53 2010 +0800 ACPI, APEI, use raw spinlock in ERST ERST writing may be used in NMI or Machine Check Exception handler. So it need to use raw spinlock instead of normal spinlock. This patch fixes it. Signed-off-by: Huang Ying Signed-off-by: Len Brown turns erst_lock from a regular spinlock into a raw spinlock. This change does not help address the fundamental problem of locking in NMI handlers - NMIs can interrupt anything, including the critical section of the lock the handler wants to acquire. The change does, however, introduce an illegal lock nesting pattern - while holding the erst_lock, there is a code path which calls printk, which may acquire a regular spinlock. This issue was caught by lockdep, and the report follows. Revert 3b38bb5f7f06 to address the issue. [ BUG: Invalid wait context ] 6.5.0-iopolicies+ #4 Not tainted ----------------------------- systemd/1 is trying to lock: ffffffff9ec053b8 (&port_lock_key){....}-{3:3}, at: serial8250_console_write+0x325/0x440 other info that might help us debug this: context-{5:5} 7 locks held by systemd/1: stack backtrace: CPU: 2 PID: 1 Comm: systemd Not tainted 6.5.0-iopolicies+ #4 Hardware name: /0JP31P, BIOS 2.7.0 05/23/2018 Call Trace: dump_stack_lvl+0x63/0xb0 __lock_acquire+0x434/0xd30 lock_acquire+0xcd/0x2b0 ? serial8250_console_write+0x325/0x440 ? __lock_acquire+0xab0/0xd30 _raw_spin_lock_irqsave+0x39/0x60 ? serial8250_console_write+0x325/0x440 serial8250_console_write+0x325/0x440 ? console_flush_all+0x1b5/0x590 console_flush_all+0x1ec/0x590 ? console_flush_all+0x184/0x590 console_unlock+0x7c/0xd0 vprintk_emit+0x261/0x380 _printk+0x5c/0x80 erst_read+0x1f8/0x210 ? erst_get_record_id_next+0x26f/0x340 erst_read_record+0x29/0xa0 erst_reader+0xad/0x2e0 ? __pfx_read_tsc+0x10/0x10 ? ktime_get_real_fast_ns+0x47/0xa0 ? __pfx_pstore_fill_super+0x10/0x10 pstore_get_backend_records+0xc6/0x2d0 ? __pfx_pstore_fill_super+0x10/0x10 pstore_get_records+0x2b/0x50 pstore_fill_super+0xb7/0xd0 mount_single+0x90/0xc0 ? security_capable+0x3c/0x60 legacy_get_tree+0x34/0x60 vfs_get_tree+0x2c/0xe0 path_mount+0x703/0xa90 do_mount+0x7d/0xa0 __x64_sys_mount+0xdc/0x100 do_syscall_64+0x3e/0x90 entry_SYSCALL_64_after_hwframe+0x6e/0xd8 RIP: 0033:0x7f3fdafa42be Code: 48 8b 0d cd 4b 38 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 9a 4b 38 00 f7 d8 64 89 01 48 RSP: 002b:00007ffdf2b2be08 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5 RAX: ffffffffffffffda RBX: 000055b567f49e78 RCX: 00007f3fdafa42be RDX: 000055b567d0294a RSI: 000055b567d02942 RDI: 000055b567d0294a RBP: 0000000000000007 R08: 0000000000000000 R09: 0000000000000002 R10: 000000000000000e R11: 0000000000000246 R12: 0000000000000000 R13: 0000000000000000 R14: 0000000000000000 R15: 00007ffdf2b2c050 Signed-off-by: Uday Shankar Reviewed-by: Joern Engel --- drivers/acpi/apei/erst.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c index 247989060e29..b2c806840a11 100644 --- a/drivers/acpi/apei/erst.c +++ b/drivers/acpi/apei/erst.c @@ -78,7 +78,7 @@ static struct erst_erange { * It is used to provide exclusive accessing for ERST Error Log * Address Range too. */ -static DEFINE_RAW_SPINLOCK(erst_lock); +static DEFINE_SPINLOCK(erst_lock); static inline int erst_errno(int command_status) { @@ -413,9 +413,9 @@ ssize_t erst_get_record_count(void) if (erst_disable) return -ENODEV; - raw_spin_lock_irqsave(&erst_lock, flags); + spin_lock_irqsave(&erst_lock, flags); count = __erst_get_record_count(); - raw_spin_unlock_irqrestore(&erst_lock, flags); + spin_unlock_irqrestore(&erst_lock, flags); return count; } @@ -480,9 +480,9 @@ static int __erst_record_id_cache_add_one(void) id = prev_id = first_id = APEI_ERST_INVALID_RECORD_ID; retry: - raw_spin_lock_irqsave(&erst_lock, flags); + spin_lock_irqsave(&erst_lock, flags); rc = __erst_get_next_record_id(&id); - raw_spin_unlock_irqrestore(&erst_lock, flags); + spin_unlock_irqrestore(&erst_lock, flags); if (rc == -ENOENT) return 0; if (rc) @@ -777,17 +777,17 @@ int erst_write(const struct cper_record_header *record) return -EINVAL; if (erst_erange.attr & ERST_RANGE_NVRAM) { - if (!raw_spin_trylock_irqsave(&erst_lock, flags)) + if (!spin_trylock_irqsave(&erst_lock, flags)) return -EBUSY; rc = __erst_write_to_nvram(record); - raw_spin_unlock_irqrestore(&erst_lock, flags); + spin_unlock_irqrestore(&erst_lock, flags); return rc; } if (record->record_length > erst_erange.size) return -EINVAL; - if (!raw_spin_trylock_irqsave(&erst_lock, flags)) + if (!spin_trylock_irqsave(&erst_lock, flags)) return -EBUSY; memcpy(erst_erange.vaddr, record, record->record_length); rcd_erange = erst_erange.vaddr; @@ -795,7 +795,7 @@ int erst_write(const struct cper_record_header *record) memcpy(&rcd_erange->persistence_information, "ER", 2); rc = __erst_write_to_storage(0); - raw_spin_unlock_irqrestore(&erst_lock, flags); + spin_unlock_irqrestore(&erst_lock, flags); return rc; } @@ -849,9 +849,9 @@ ssize_t erst_read(u64 record_id, struct cper_record_header *record, if (erst_disable) return -ENODEV; - raw_spin_lock_irqsave(&erst_lock, flags); + spin_lock_irqsave(&erst_lock, flags); len = __erst_read(record_id, record, buflen); - raw_spin_unlock_irqrestore(&erst_lock, flags); + spin_unlock_irqrestore(&erst_lock, flags); return len; } EXPORT_SYMBOL_GPL(erst_read); @@ -936,12 +936,12 @@ int erst_clear(u64 record_id) rc = mutex_lock_interruptible(&erst_record_id_cache.lock); if (rc) return rc; - raw_spin_lock_irqsave(&erst_lock, flags); + spin_lock_irqsave(&erst_lock, flags); if (erst_erange.attr & ERST_RANGE_NVRAM) rc = __erst_clear_from_nvram(record_id); else rc = __erst_clear_from_storage(record_id); - raw_spin_unlock_irqrestore(&erst_lock, flags); + spin_unlock_irqrestore(&erst_lock, flags); if (rc) goto out; entries = erst_record_id_cache.entries;