lkml.org 
[lkml]   [2017]   [Jul]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v6 RESEND] x86/boot/KASLR: Restrict kernel to be randomized in mirror regions

* Baoquan He <bhe@redhat.com> wrote:

> +/*
> + * Returns true if mirror region found (and must have been processed
> + * for slots adding)
> + */
> +static bool process_efi_entries(unsigned long minimum,
> + unsigned long image_size)

Also, please don't break the line in the middle of the prototype.

> +{
> + struct efi_info *e = &boot_params->efi_info;
> + bool efi_mirror_found = false;
> + struct mem_vector region;
> + efi_memory_desc_t *md;
> + unsigned long pmap;
> + char *signature;
> + u32 nr_desc;
> + int i;
> +
> + signature = (char *)&boot_params->efi_info.efi_loader_signature;

This is sloppy too: we already have '&boot_params->efi_info' in 'e', why do you
duplicate it again, why not write 'e->efi_loader_signature'??

> + if (strncmp(signature, EFI32_LOADER_SIGNATURE, 4) &&
> + strncmp(signature, EFI64_LOADER_SIGNATURE, 4))
> + return false;
> +
> +#ifdef CONFIG_X86_32
> + /* Can't handle data above 4GB at this time */
> + if (e->efi_memmap_hi) {
> + warn("Memory map is above 4GB, EFI should be disabled.\n");
> + return false;

This kernel warning is pretty passive-aggressive: please explain what the problem
is and how it can be resolved.

> + }
> + pmap = e->efi_memmap;
> +#else
> + pmap = (e->efi_memmap | ((__u64)e->efi_memmap_hi << 32));
> +#endif
> +
> + nr_desc = e->efi_memmap_size / e->efi_memdesc_size;
> + for (i = 0; i < nr_desc; i++) {
> + md = (efi_memory_desc_t *)(pmap + (i * e->efi_memdesc_size));

This looks unnecessarily obfuscated: why not initialize 'md' to 'pmap' when pmap
is calculated and just use md[i]?

> +static inline bool process_efi_entries(unsigned long minimum,
> + unsigned long image_size)

ugly linebreak again ...

Thanks,

Ingo

\
 
 \ /
  Last update: 2017-07-21 12:39    [W:0.131 / U:2.412 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site