| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20230822010933.2247671-1-ushankar@purestorage.com>
Date: Mon, 21 Aug 2023 19:09:34 -0600
From: Uday Shankar <ushankar@...estorage.com>
To: Joern Engel <joern@...estorage.com>,
Kees Cook <keescook@...omium.org>,
Tony Luck <tony.luck@...el.com>,
"Guilherme G. Piccoli" <gpiccoli@...lia.com>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Len Brown <lenb@...nel.org>, James Morse <james.morse@....com>,
Borislav Petkov <bp@...en8.de>,
Huang Ying <ying.huang@...el.com>,
Len Brown <len.brown@...el.com>
Cc: Uday Shankar <ushankar@...estorage.com>,
linux-hardening@...r.kernel.org, linux-acpi@...r.kernel.org
Subject: [PATCH] Revert "ACPI, APEI, use raw spinlock in ERST"
The commit
commit 3b38bb5f7f06356a89bb2cbf92ad346025e95192
Author: Huang Ying <ying.huang@...el.com>
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 <ying.huang@...el.com>
Signed-off-by: Len Brown <len.brown@...el.com>
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:
<TASK>
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
</TASK>
Signed-off-by: Uday Shankar <ushankar@...estorage.com>
Reviewed-by: Joern Engel <joern@...estorage.com>
---
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;
--
2.25.1
Powered by blists - more mailing lists