EFI-pstore, sleeping from back context after fault

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



So I triggered a bug in x86 code. First the "okay" backtrace:
|BUG: GPF in non-whitelisted uaccess (non-canonical address?)
|general protection fault: 0000 [#1] PREEMPT SMP NOPTI
|CPU: 26 PID: 2236 Comm: sig-xstate-bum Not tainted 4.20.0-rc3 #45
|RIP: 0010:__fpu__restore_sig+0x1c1/0x540
|Call Trace:
| fpu__restore_sig+0x28/0x40
| restore_sigcontext+0x13a/0x180
| __ia32_sys_rt_sigreturn+0xae/0x100
| do_syscall_64+0x4f/0x100
| entry_SYSCALL_64_after_hwframe+0x44/0xa9
|RIP: 0033:0x7f9b06aea227
|---[ end trace a45ac23b593e9ab0 ]---

and now the not okay part:

|BUG: sleeping function called from invalid context at kernel/sched/completion.c:99
|in_atomic(): 1, irqs_disabled(): 1, pid: 2236, name: sig-xstate-bum
|Preemption disabled at:
|[<ffffffff99d60512>] pstore_dump+0x72/0x330
|CPU: 26 PID: 2236 Comm: sig-xstate-bum Tainted: G      D           4.20.0-rc3 #45
|Call Trace:
| dump_stack+0x4f/0x6a
| ___might_sleep.cold.91+0xd3/0xe4
| __might_sleep+0x50/0x90
| wait_for_completion+0x32/0x130
| virt_efi_query_variable_info+0x14e/0x160
| efi_query_variable_store+0x51/0x1a0
| efivar_entry_set_safe+0xa3/0x1b0
| efi_pstore_write+0x109/0x140
| pstore_dump+0x11c/0x330
| kmsg_dump+0xa4/0xd0
| oops_exit+0x22/0x30
| oops_end+0x67/0xd0
| die+0x41/0x4a
| do_general_protection+0xc1/0x150
| general_protection+0x1e/0x30
|RIP: 0010:__fpu__restore_sig+0x1c1/0x540
| fpu__restore_sig+0x28/0x40
| restore_sigcontext+0x13a/0x180
| __ia32_sys_rt_sigreturn+0xae/0x100
| do_syscall_64+0x4f/0x100
| entry_SYSCALL_64_after_hwframe+0x44/0xa9
|RIP: 0033:0x7f9b06aea227
… [ moar backtrace ]

This looks like it comes from commit 21b3ddd39fee ("efi: Don't use
spinlocks for efi vars") because it replaced a spin_lock() with
down_interruptible() in this case. In this case, since pstore_dump()
holds psinfo->buf_lock with disabled interrupts we can't block…

I have this as a workaround:

diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c
index 9336ffdf6e2c..d4dcfe46eb2e 100644
--- a/drivers/firmware/efi/vars.c
+++ b/drivers/firmware/efi/vars.c
@@ -755,7 +755,9 @@ int efivar_entry_set_safe(efi_char16_t *name, efi_guid_t vendor, u32 attributes,
 			return -EINTR;
 	}
 
-	status = check_var_size(attributes, size + ucs2_strsize(name, 1024));
+	status = ops->query_variable_store(attributes,
+					   size + ucs2_strsize(name, 1024),
+					   block);
 	if (status != EFI_SUCCESS) {
 		up(&efivars_lock);
 		return -ENOSPC;
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index b821054ca3ed..9aa27a2c8d36 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -127,10 +127,10 @@ static const char *get_reason_str(enum kmsg_dump_reason reason)
 bool pstore_cannot_block_path(enum kmsg_dump_reason reason)
 {
 	/*
-	 * In case of NMI path, pstore shouldn't be blocked
-	 * regardless of reason.
+	 * In case of disabled preemption / interrupts we can't block on any
+	 * lock regardless of reason.
 	 */
-	if (in_nmi())
+	if (in_atomic() || irqs_disabled())
 		return true;
 
 	switch (reason) {

Sebastian



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux