Hello Gen, Thanks for the patch. On Fri, 17 May 2019 at 10:26, Gen Zhang <blackgod016574@xxxxxxxxx> wrote: > > save_pgd is allocated by kmalloc_array. And it is dereferenced in the > following codes. However, memory allocation functions such as > kmalloc_array may fail. Dereferencing this save_pgd null pointer may > cause the kernel go wrong. Thus we should check this allocation and add > error handling code. > > Signed-off-by: Gen Zhang <blackgod016574@xxxxxxxxx> > > --- > diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c > index cf0347f..fb9ae57 100644 > --- a/arch/x86/platform/efi/efi_64.c > +++ b/arch/x86/platform/efi/efi_64.c > @@ -91,6 +91,8 @@ pgd_t * __init efi_call_phys_prolog(void) > > n_pgds = DIV_ROUND_UP((max_pfn << PAGE_SHIFT), PGDIR_SIZE); > save_pgd = kmalloc_array(n_pgds, sizeof(*save_pgd), GFP_KERNEL); > + if (!save_pgd) > + goto err; > > /* > * Build 1:1 identity mapping for efi=old_map usage. Note that > @@ -142,6 +144,9 @@ pgd_t * __init efi_call_phys_prolog(void) > __flush_tlb_all(); > > return save_pgd; > +err: > + __flush_tlb_all(); What is the point of the goto and the TLB flush? > + return ERR_PTR(-ENOMEM); Returning an error here is not going to make much difference, given that the caller of efi_call_phys_prolog() does not bother to check it, and passes the result straight into efi_call_phys_epilog(), which happily attempts to dereference it. So if you want to fix this properly, please fix it at the call site as well. I'd prefer to avoid ERR_PTR() and just return NULL for a failed allocation though. > } > > void __init efi_call_phys_epilog(pgd_t *save_pgd) > ---
