From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754427AbdHYClV (ORCPT ); Thu, 24 Aug 2017 22:41:21 -0400 Received: from mga06.intel.com ([134.134.136.31]:12226 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754216AbdHYClT (ORCPT ); Thu, 24 Aug 2017 22:41:19 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,423,1498546800"; d="scan'208";a="141806413" Message-ID: <1503628588.30475.61.camel@intel.com> Subject: Re: [PATCH 3/3] x86/efi: Use efi_switch_mm() rather than manually twiddling with cr3 From: Sai Praneeth Prakhya To: Andy Lutomirski Cc: 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" Date: Thu, 24 Aug 2017 19:36:28 -0700 In-Reply-To: References: <1502824706-30762-1-git-send-email-sai.praneeth.prakhya@intel.com> <1502824706-30762-4-git-send-email-sai.praneeth.prakhya@intel.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.12.11-0ubuntu3 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. 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