From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933760AbdHYPOK (ORCPT ); Fri, 25 Aug 2017 11:14:10 -0400 Received: from mail.kernel.org ([198.145.29.99]:33416 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933494AbdHYPOI (ORCPT ); Fri, 25 Aug 2017 11:14:08 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 95B7D21A6C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=luto@kernel.org MIME-Version: 1.0 In-Reply-To: <1503628588.30475.61.camel@intel.com> References: <1502824706-30762-1-git-send-email-sai.praneeth.prakhya@intel.com> <1502824706-30762-4-git-send-email-sai.praneeth.prakhya@intel.com> <1503628588.30475.61.camel@intel.com> From: Andy Lutomirski Date: Fri, 25 Aug 2017 08:13:46 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 3/3] x86/efi: Use efi_switch_mm() rather than manually twiddling with cr3 To: Sai Praneeth Prakhya Cc: Andy Lutomirski , Peter Zijlstra , "linux-efi@vger.kernel.org" , "linux-kernel@vger.kernel.org" , joeyli , Borislav Petkov , "Michael S. Tsirkin" , "Neri, Ricardo" , Matt Fleming , Ard Biesheuvel , "Shankar, Ravi V" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Aug 24, 2017 at 7:36 PM, Sai Praneeth Prakhya wrote: > On Tue, 2017-08-15 at 14:46 -0700, Andy Lutomirski wrote: >> On Tue, Aug 15, 2017 at 12:18 PM, Sai Praneeth Prakhya >> wrote: >> > +/* >> > + * Makes the calling kernel thread switch to/from efi_mm context >> > + * Can be used from SetVirtualAddressMap() or during efi runtime calls >> > + * (Note: This routine is heavily inspired from use_mm) >> > + */ >> > +void efi_switch_mm(struct mm_struct *mm) >> > +{ >> > + struct task_struct *tsk = current; >> > + >> > + task_lock(tsk); >> > + efi_scratch.prev_mm = tsk->active_mm; >> > + if (efi_scratch.prev_mm != mm) { >> > + mmgrab(mm); >> > + tsk->active_mm = mm; >> > + } >> > + switch_mm(efi_scratch.prev_mm, mm, NULL); >> > + task_unlock(tsk); >> > + >> > + if (efi_scratch.prev_mm != mm) >> > + mmdrop(efi_scratch.prev_mm); >> >> I'm confused. You're mmdropping an mm that you are still keeping a >> pointer to. This is also a bit confusing in the case where you do >> efi_switch_mm(efi_scratch.prev_mm). >> >> This whole manipulation seems fairly dangerous to me for another >> reason -- you're taking a user thread (I think) and swapping out its >> mm to something that the user in question should *not* have access to. >> What if a perf interrupt happens while you're in the alternate mm? >> What if you segfault and dump core? Should we maybe just have a flag >> that says "this cpu is using a funny mm", assert that the flag is >> clear when scheduling, and teach perf, coredumps, etc not to touch >> user memory when the flag is set? >> >> Admittedly, the latter problem may well have existed even before these patches. > > Hi All, > > Could we please decouple the above issue from this patch set, so that we > could have common efi_mm between x86 and ARM and also improve > readability and maintainability for x86/efi. I don't see why not. > > As it seems that "Everything EFI as kthread" might solve the above issue > for real (which might take quite some time to implement, taking into > consideration the complexity involved and some special case with > pstore), do you think this patch set seems OK? > > If so, I will send out a V2 addressing the mmdropping issue. > > Regards, > Sai >