linux-kernel - [PATCH 08/11] efi/x86: drop task_lock() from efi_switch

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: <20180927085039.8391-9-ard.biesheuvel@linaro.org>
Date:   Thu, 27 Sep 2018 10:50:35 +0200
From:   Ard Biesheuvel <ard.biesheuvel@...aro.org>
To:     linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>
Cc:     Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        linux-efi@...r.kernel.org, Aaron Ma <aaron.ma@...onical.com>,
        Alistair Strachan <astrachan@...gle.com>,
        Ben Hutchings <ben@...adent.org.uk>,
        Bhupesh Sharma <bhsharma@...hat.com>,
        Hans de Goede <hdegoede@...hat.com>,
        Ivan Hu <ivan.hu@...onical.com>,
        Jeremy Linton <jeremy.linton@....com>,
        Marc Zyngier <marc.zyngier@....com>,
        Matt Fleming <matt@...eblueprint.co.uk>,
        Peter Robinson <pbrobinson@...hat.com>,
        Sai Praneeth Prakhya <sai.praneeth.prakhya@...el.com>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Stefan Agner <stefan@...er.ch>
Subject: [PATCH 08/11] efi/x86: drop task_lock() from efi_switch_mm()

From: Sebastian Andrzej Siewior <bigeasy@...utronix.de>

efi_switch_mm() is a wrapper around switch_mm() which saves current's
->active_mm, sets the requests mm as ->active_mm and invokes
switch_mm().
I don't think that task_lock() is required during that procedure. It
protects ->mm which isn't changed here.

It needs to be mentioned that during the whole procedure (switch to
EFI's mm and back) the preemption needs to be disabled. A context switch
at this point would reset the cr3 value based on current->mm. Also, this
function may not be invoked at the same time on a different CPU because
it would overwrite the efi_scratch.prev_mm information.

Remove task_lock() and also update the comment to reflect it.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@...aro.org>
---
 arch/x86/platform/efi/efi_64.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index ee5d08f25ce4..e8da7f492970 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -619,18 +619,16 @@ void __init efi_dump_pagetable(void)
 
 /*
  * Makes the calling thread switch to/from efi_mm context. Can be used
- * for SetVirtualAddressMap() i.e. current->active_mm == init_mm as well
- * as during efi runtime calls i.e current->active_mm == current_mm.
- * We are not mm_dropping()/mm_grabbing() any mm, because we are not
- * losing/creating any references.
+ * in a kernel thread and user context. Preemption needs to remain disabled
+ * while the EFI-mm is borrowed. mmgrab()/mmdrop() is not used because the mm
+ * can not change under us.
+ * It should be ensured that there are no concurent calls to this function.
  */
 void efi_switch_mm(struct mm_struct *mm)
 {
-	task_lock(current);
 	efi_scratch.prev_mm = current->active_mm;
 	current->active_mm = mm;
 	switch_mm(efi_scratch.prev_mm, mm, NULL);
-	task_unlock(current);
 }
 
 #ifdef CONFIG_EFI_MIXED
-- 
2.18.0