linux-kernel - Re: [GIT PULL] EFI changes for v3.18

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140929124321.GB18825@gmail.com>
Date:	Mon, 29 Sep 2014 14:43:21 +0200
From:	Ingo Molnar <mingo@...nel.org>
To:	Matt Fleming <matt@...sole-pimps.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Cc:	"H. Peter Anvin" <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	linux-efi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL] EFI changes for v3.18


* Matt Fleming <matt@...sole-pimps.org> wrote:

>  * Implement new EFI runtime lock which is required by the UEFI
>    specification - Ard Biesheuvel

Firstly, under what circumstances can EFI call parallelism happen 
currently? Most of the EFI code runs during early bootup, which 
is serialized.

Secondly, this locking pattern looks pretty disgusting:

@@ -94,7 +187,17 @@ static efi_status_t virt_efi_set_variable(efi_char16_t *name,
                                          unsigned long data_size,
                                          void *data)
 {
-       return efi_call_virt(set_variable, name, vendor, attr, data_size, data);
+       unsigned long flags;
+       efi_status_t status;
+       bool __in_nmi = efi_in_nmi();
+
+       if (!__in_nmi)
+               spin_lock_irqsave(&efi_runtime_lock, flags);
+       status = efi_call_virt(set_variable, name, vendor, attr, data_size,
+                              data);
+       if (!__in_nmi)
+               spin_unlock_irqrestore(&efi_runtime_lock, flags);
+       return status;
 }

and that's repeated in virt_efi_query_variable_info() as well.

and that's the explanation given:

+/*
+ * Some runtime services calls can be reentrant under NMI, even if the table
+ * above says they are not. (source: UEFI Specification v2.4A)
+ *
+ * Table 32. Functions that may be called after Machine Check, INIT and NMI
+ * 
+----------------------------+------------------------------------------+
+ * | Function                  | Called after Machine Check, INIT and NMI |
+ * 
+----------------------------+------------------------------------------+
+ * | GetTime()                 | Yes, even if previously busy.            |
+ * | GetVariable()             | Yes, even if previously busy             |
+ * | GetNextVariableName()     | Yes, even if previously busy             |
+ * | QueryVariableInfo()       | Yes, even if previously busy             |
+ * | SetVariable()             | Yes, even if previously busy             |
+ * | UpdateCapsule()           | Yes, even if previously busy             |
+ * | QueryCapsuleCapabilities()| Yes, even if previously busy             |
+ * | ResetSystem()             | Yes, even if previously busy             |
+ * 
+----------------------------+------------------------------------------+
+ *
+ * In order to prevent deadlocks under NMI, the wrappers for these functions
+ * may only grab the efi_runtime_lock or rtc_lock spinlocks if !efi_in_nmi().
+ * However, not all of the services listed are reachable through NMI code paths,
+ * so the the special handling as suggested by the UEFI spec is only implemented
+ * for QueryVariableInfo() and SetVariable(), as these can be reached in NMI
+ * context through efi_pstore_write().

Are pstore calls into the EFI runtime reentrant?

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/