linux-kernel - Re: [PATCH 1/1] efi/libstub: describe efi_relocate

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: <9687832d-e9a9-ca47-34d5-7b912b2f718a@infradead.org>
Date:   Wed, 19 Feb 2020 23:02:32 -0800
From:   Randy Dunlap <rdunlap@...radead.org>
To:     Heinrich Schuchardt <xypron.glpk@....de>,
        Ard Biesheuvel <ardb@...nel.org>
Cc:     linux-efi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] efi/libstub: describe efi_relocate_kernel()

Hi,
Mostly looks good.  One comment below:

On 2/19/20 10:53 PM, Heinrich Schuchardt wrote:
> Update the description of of efi_relocate_kernel() to match Sphinx style.
> 
> Update parameter references in the description of other memory functions
> to use @param style.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@....de>
> ---
>  drivers/firmware/efi/libstub/mem.c | 38 +++++++++++++++++++-----------
>  1 file changed, 24 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/firmware/efi/libstub/mem.c b/drivers/firmware/efi/libstub/mem.c
> index 0d57078e5e62..7efe3ed2d5a6 100644
> --- a/drivers/firmware/efi/libstub/mem.c
> +++ b/drivers/firmware/efi/libstub/mem.c
> @@ -86,7 +86,7 @@ efi_status_t efi_get_memory_map(struct efi_boot_memmap *map)
>   *
>   * Allocate pages as EFI_LOADER_DATA. The allocated pages are aligned according
>   * to EFI_ALLOC_ALIGN. The last allocated page will not exceed the address
> - * given by 'max'.
> + * given by @max.
>   *
>   * Return:	status code
>   */
> @@ -126,10 +126,10 @@ efi_status_t efi_allocate_pages(unsigned long size, unsigned long *addr,
>   * @addr:	on exit the address of the allocated memory
>   * @min:	minimum address to used for the memory allocation
>   *
> - * Allocate at the lowest possible address that is not below 'min' as
> - * EFI_LOADER_DATA. The allocated pages are aligned according to 'align' but at
> + * Allocate at the lowest possible address that is not below @min as
> + * EFI_LOADER_DATA. The allocated pages are aligned according to @align but at
>   * least EFI_ALLOC_ALIGN. The first allocated page will not below the address
> - * given by 'min'.
> + * given by @min.
>   *
>   * Return:	status code
>   */
> @@ -214,7 +214,7 @@ efi_status_t efi_low_alloc_above(unsigned long size, unsigned long align,
>   * @addr:	start of the memory area to free (must be EFI_PAGE_SIZE
>   *		aligned)
>   *
> - * 'size' is rounded up to a multiple of EFI_ALLOC_ALIGN which is an
> + * @size is rounded up to a multiple of EFI_ALLOC_ALIGN which is an
>   * architecture specific multiple of EFI_PAGE_SIZE. So this function should
>   * only be used to return pages allocated with efi_allocate_pages() or
>   * efi_low_alloc_above().
> @@ -230,15 +230,25 @@ void efi_free(unsigned long size, unsigned long addr)
>  	efi_bs_call(free_pages, addr, nr_pages);
>  }
> 
> -/*
> - * Relocate a kernel image, either compressed or uncompressed.
> - * In the ARM64 case, all kernel images are currently
> - * uncompressed, and as such when we relocate it we need to
> - * allocate additional space for the BSS segment. Any low
> - * memory that this function should avoid needs to be
> - * unavailable in the EFI memory map, as if the preferred
> - * address is not available the lowest available address will
> - * be used.
> +/**
> + * efi_relocate_kernel() - copy memory area
> + * @image_addr:		address of memory area to copy, on exit target address

The "on exit target address" is a little bit confusing IMO.
Is it like this?

  On exit, @image_addr is updated to the target copy address that was used.

?  or some other better description?

Thanks.

Acked-by: Randy Dunlap <rdunlap@...radead.org>


> + * @image_size:		size of memory area to copy
> + * @alloc_size:		minimum size of memory to allocate, must be greater or
> + *			equal to image_size
> + * @preferred_addr:	preferred target address
> + * @alignment:		minimum alignment of the allocated memory area. It
> + *			should be a power of two.
> + * @min_addr:		minimum target address
> + *
> + * Copy a memory area to a newly allocated memory area aligned according
> + * to @alignment but at least EFI_ALLOC_ALIGN. If the preferred address
> + * is not available, the allocated address will not be below @min_addr.
> + *
> + * This function is used to copy the Linux kernel verbatim. It does not apply
> + * any relocation changes.
> + *
> + * Return:		status code
>   */
>  efi_status_t efi_relocate_kernel(unsigned long *image_addr,
>  				 unsigned long image_size,
> --
> 2.25.0
> 


-- 
~Randy