linux-kernel - RE: [PATCH] x86/kaslr: try process e820 entries if can not get suitable regions from efi

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: <790039cd9c374a678abcb286d2709d2b@huawei.com>
Date:   Wed, 6 Jan 2021 03:04:04 +0000
From:   "linfeng (M)" <linfeng23@...wei.com>
To:     Arvind Sankar <nivedita@...m.mit.edu>,
        Ard Biesheuvel <ardb@...nel.org>
CC:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "Huangweidong (C)" <weidong.huang@...wei.com>,
        "Wangjing (Hogan, Cloud Infrastructure Service Product Dept.)" 
        <hogan.wang@...wei.com>,
        "Wangxin (Alexander)" <wangxinxin.wang@...wei.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "Eric W. Biederman" <ebiederm@...ssion.com>
Subject: RE: [PATCH] x86/kaslr: try process e820 entries if can not get
 suitable regions from efi



> -----Original Message-----
> From: Arvind Sankar [mailto:niveditas98@...il.com] On Behalf Of Arvind
> Sankar
> Sent: Wednesday, January 6, 2021 5:01 AM
> To: Ard Biesheuvel <ardb@...nel.org>
> Cc: linfeng (M) <linfeng23@...wei.com>; Arvind Sankar
> <nivedita@...m.mit.edu>; Thomas Gleixner <tglx@...utronix.de>; Ingo Molnar
> <mingo@...hat.com>; Borislav Petkov <bp@...en8.de>; Huangweidong (C)
> <weidong.huang@...wei.com>; Wangjing (Hogan, Cloud Infrastructure
> Service Product Dept.) <hogan.wang@...wei.com>; Wangxin (Alexander)
> <wangxinxin.wang@...wei.com>; Linux Kernel Mailing List
> <linux-kernel@...r.kernel.org>; Eric W. Biederman <ebiederm@...ssion.com>
> Subject: Re: [PATCH] x86/kaslr: try process e820 entries if can not get suitable
> regions from efi
> 
> On Tue, Jan 05, 2021 at 09:54:52AM +0100, Ard Biesheuvel wrote:
> > (cc Arvind)
> >
> > On Tue, 5 Jan 2021 at 09:54, Lin Feng <linfeng23@...wei.com> wrote:
> > >
> > > On efi64 x86_64 system, the EFI_CONVENTIONAL_MEMORY regions will
> not
> > > be mapped when making EFI runtime calls. So kexec-tools can not get
> > > these from /sys/firmware/efi/runtime-map. Then compressed boot os
> > > can not get suitable regions in process_efi_entries and print debug
> > > message as follow:
> > >         Physical KASLR disabled: no suitable memory region!
> > > To enable physical kaslr with kexec, call process_e820_entries when
> > > no suitable regions in efi memmaps.
> > >
> > > Signed-off-by: Lin Feng <linfeng23@...wei.com>
> > >
> > > ---
> > >
> > > I find a regular of Kernel code and data placement with kexec. It
> > > seems unsafe. The reason is showed above.
> > >
> > > I'm not familiar with efi firmware. I wonder if there are some risks
> > > to get regions according to e820 when there is no suitable region in
> > > efi memmaps.
> > > ---
> > >  arch/x86/boot/compressed/kaslr.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/x86/boot/compressed/kaslr.c
> > > b/arch/x86/boot/compressed/kaslr.c
> > > index b92fffbe761f..dbd7244b71aa 100644
> > > --- a/arch/x86/boot/compressed/kaslr.c
> > > +++ b/arch/x86/boot/compressed/kaslr.c
> > > @@ -685,6 +685,7 @@ process_efi_entries(unsigned long minimum,
> > > unsigned long image_size)  {
> > >         struct efi_info *e = &boot_params->efi_info;
> > >         bool efi_mirror_found = false;
> > > +       bool efi_mem_region_found = false;
> > >         struct mem_vector region;
> > >         efi_memory_desc_t *md;
> > >         unsigned long pmap;
> > > @@ -742,12 +743,13 @@ process_efi_entries(unsigned long minimum,
> unsigned long image_size)
> > >                     !(md->attribute &
> EFI_MEMORY_MORE_RELIABLE))
> > >                         continue;
> > >
> > > +               efi_mem_region_found = false;
> 					   ^^ this should be true, not false.
You're right. It should be true here. Thanks for pointing out.
> 
> Other than that, I think this should be okay. The reason EFI memmap is
> preferred over E820, according to commit
> 
>   0982adc74673 ("x86/boot/KASLR: Work around firmware bugs by excluding
> EFI_BOOT_SERVICES_* and EFI_LOADER_* from KASLR's choice")
> 
> was to avoid allocating inside EFI_BOOT_SERVICES/EFI_LOADER_DATA etc.
> That's not a danger during kexec, and I believe runtime services regions should
> be marked as reserved in the E820 map, right?
Yes.
> 
> Also, something a little fishy-looking here is that the first loop to see if there is
> any EFI_MEMORY_MORE_RELIABLE region does not apply any of the checks on
> the memory region type/attributes. If there is a mirror region but it isn't
> conventional memory, or if it was soft-reserved, we shouldn't be setting
> efi_mirror_found.
I think so. And I wonder if the memory mirror doesn't work with kexec and ksalr
only this patch used, because a lot of efi information is lost and e820 don't have any
mirror regions information. Due to resource constraints, I haven't tested it yet.
But it seems so.
> 
> 
> > >                 region.start = md->phys_addr;
> > >                 region.size = md->num_pages << EFI_PAGE_SHIFT;
> > >                 if (process_mem_region(&region, minimum,
> image_size))
> > >                         break;
> > >         }
> > > -       return true;
> > > +       return efi_mem_region_found;
> > >  }
> > >  #else
> > >  static inline bool
> > > --
> > > 2.23.0
> > >