On Tue, 4 Jul 2017, Baoquan He wrote:
> On 07/04/17 at 04:00pm, Thomas Gleixner wrote:
> > On Tue, 4 Jul 2017, Baoquan He wrote:
> > > +/* Marks if efi mirror regions have been found and handled. */
> > > +static bool efi_mirror_found;
> > > +
> > > +static void process_efi_entry(unsigned long minimum, unsigned long
> > > image_size)
> > > +{
> > > + struct efi_info *e = &boot_params->efi_info;
> > > + struct mem_vector region;
> > > + efi_memory_desc_t *md;
> > > + unsigned long pmap;
> > > + char *signature;
> > > + u32 nr_desc;
> > > + int i;
> > > +
> > > +
> > > +#ifdef CONFIG_EFI
> > > + signature = (char *)&boot_params->efi_info.efi_loader_signature;
> > > +#endif
> >
> > So if CONFIG_EFI=n you happily dereference the uninitialized signature
> > pointer ...
>
> Right, this is a mistake. Thanks for pointing it out. I should have
> checked if the pointer is NULL.
The pointer is not NULL, it's not initialized.
> In fact I just referred to code in setup_arch(). Now I have a question,
> though CONFIG_EFI=y but efi firmware is not enabled,
> boot_params.efi_info.efi_loader_signature should be initilized to 0.
> Then below code is also problematic.
>
> #ifdef CONFIG_EFI
> if (!strncmp((char *)&boot_params.efi_info.efi_loader_signature,
>
> EFI32_LOADER_SIGNATURE, 4)) {
> set_bit(EFI_BOOT, &efi.flags);
> } else if (!strncmp((char
> *)&boot_params.efi_info.efi_loader_signature,
> EFI64_LOADER_SIGNATURE, 4)) {
> set_bit(EFI_BOOT, &efi.flags);
> set_bit(EFI_64BIT, &efi.flags);
> }
>
> if (efi_enabled(EFI_BOOT))
> efi_memblock_x86_reserve_range();
> #endif
Indeed. Matt?
Thanks,
tglx
