On 21 February 2012 10:42, Damian, Alexandru <alexandru.dam...@intel.com> wrote:
> Hi,
>
> I got a problem with QEMU refusing to load an ELF binary with 0-length
> sections,
> while the kernel has no issue doing this.
>
> This patch adds a check that has been in kernel since 2008 at least.
CC'ing Riku (linux-user maintainer).
>
> Cheers,
> Alex
>
> ----------------
> commit a42e5231c1be5f09caeb6c73e34933cd7efa7023
> Author: Alexandru DAMIAN <alexandru.dam...@intel.com>
> Date: Tue Feb 21 12:34:36 2012 +0200
>
> Do not attempt to map 0-length sections
>
> Mmap will return an invalid argument, but 0-length sections
> are valid in any case. The kernel as a similar check
> when loading elf binaries courtesy of jkos...@suse.cz.
>
> Signed-off-by: Alexandru Damian <alexandru.dam...@intel.com>
Convention is to submit patches as git-format-patch style emails.
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index ea61d0d..71d0ae3 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -918,9 +918,9 @@ static inline void init_thread(struct target_pt_regs
> *regs,
>
> #define elf_check_arch(x) ( (x) == ELF_ARCH )
>
> -#define ELF_CLASS ELFCLASS64
> -#define ELF_DATA ELFDATA2MSB
> -#define ELF_ARCH EM_S390
> +#define ELF_CLASS ELFCLASS64
> +#define ELF_DATA ELFDATA2MSB
> +#define ELF_ARCH EM_S390
Please don't make unrelated whitespace changes.
>
> static inline void init_thread(struct target_pt_regs *regs, struct
> image_info *infop)
> {
> @@ -1565,11 +1565,16 @@ static void load_elf_image(const char *image_name,
> int image_fd,
> vaddr_po = TARGET_ELF_PAGEOFFSET(vaddr);
> vaddr_ps = TARGET_ELF_PAGESTART(vaddr);
>
> - error = target_mmap(vaddr_ps, eppnt->p_filesz + vaddr_po,
> - elf_prot, MAP_PRIVATE | MAP_FIXED,
> - image_fd, eppnt->p_offset - vaddr_po);
> - if (error == -1) {
> - goto exit_perror;
> + /* Don't attempt to map 0 bytes len sections.
> + Kernel also has this check.
> + */
This comment could be better:
/* mmap() will fail EINVAL if given a zero size, but a
* segment with zero filesize is perfectly valid (and
* handled by the kernel's ELF loading code).
*/
> + if (eppnt->p_filesz + vaddr_po != 0) {
> + error = target_mmap(vaddr_ps, eppnt->p_filesz +
> vaddr_po,
> + elf_prot, MAP_PRIVATE | MAP_FIXED,
> + image_fd, eppnt->p_offset -
> vaddr_po);
Something in your patch submission path is wrapping long lines.
> + if (error == -1) {
> + goto exit_perror;
> + }
QEMU coding style is four-space indents, but the indent in your new
code is mostly eight-space.
> }
>
> vaddr_ef = vaddr + eppnt->p_filesz;
>
Otherwise looks OK.
-- PMM
