All of lore.kernel.org
 help / color / mirror / Atom feed
From: Borislav Petkov <bp@alien8.de>
To: Vivek Goyal <vgoyal@redhat.com>
Cc: mjg59@srcf.ucam.org, jkosina@suse.cz, greg@kroah.com,
	kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
	ebiederm@xmission.com, hpa@zytor.com
Subject: Re: [PATCH 10/11] kexec: Support for loading ELF x86_64 images
Date: Fri, 28 Feb 2014 15:58:32 +0100	[thread overview]
Message-ID: <20140228145832.GF4326@pd.tnic> (raw)
In-Reply-To: <1390849071-21989-11-git-send-email-vgoyal@redhat.com>

On Mon, Jan 27, 2014 at 01:57:50PM -0500, Vivek Goyal wrote:
> This patch provides support for kexec for loading ELF x86_64 images. I have
> tested it with loading vmlinux and it worked.

Can you please enlighten me what the use case for ELF kernel images is? bzImage
I understand but what produces ELF images?

I see that kexec_file_load() can receive ELF segments too but why are we
doing that?

> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> ---
>  arch/x86/include/asm/kexec-elf.h   |  11 ++
>  arch/x86/kernel/Makefile           |   1 +
>  arch/x86/kernel/kexec-elf.c        | 231 +++++++++++++++++++++++++++++++++++++
>  arch/x86/kernel/machine_kexec_64.c |   2 +
>  4 files changed, 245 insertions(+)
>  create mode 100644 arch/x86/include/asm/kexec-elf.h
>  create mode 100644 arch/x86/kernel/kexec-elf.c
> 
> diff --git a/arch/x86/include/asm/kexec-elf.h b/arch/x86/include/asm/kexec-elf.h
> new file mode 100644
> index 0000000..afef382
> --- /dev/null
> +++ b/arch/x86/include/asm/kexec-elf.h
> @@ -0,0 +1,11 @@
> +#ifndef _ASM_KEXEC_ELF_H
> +#define _ASM_KEXEC_ELF_H
> +
> +extern int elf_x86_64_probe(const char *buf, unsigned long len);
> +extern void *elf_x86_64_load(struct kimage *image, char *kernel,
> +		unsigned long kernel_len, char *initrd,
> +		unsigned long initrd_len, char *cmdline,
> +		unsigned long cmdline_len);
> +extern int elf_x86_64_cleanup(struct kimage *image);
> +
> +#endif  /* _ASM_KEXEC_ELF_H */
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index fa9981d..2d77de7 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -71,6 +71,7 @@ obj-$(CONFIG_KEXEC)		+= machine_kexec.o
>  obj-$(CONFIG_KEXEC)		+= machine_kexec_$(BITS).o
>  obj-$(CONFIG_KEXEC)		+= relocate_kernel_$(BITS).o crash.o
>  obj-$(CONFIG_KEXEC)		+= kexec-bzimage.o
> +obj-$(CONFIG_KEXEC)		+= kexec-elf.o

It looks like kexec could slowly grow its own dir now:

arch/x86/kexec/

or so.

>  obj-$(CONFIG_CRASH_DUMP)	+= crash_dump_$(BITS).o
>  obj-y				+= kprobes/
>  obj-$(CONFIG_MODULES)		+= module.o
> diff --git a/arch/x86/kernel/kexec-elf.c b/arch/x86/kernel/kexec-elf.c
> new file mode 100644
> index 0000000..ff1017c
> --- /dev/null
> +++ b/arch/x86/kernel/kexec-elf.c
> @@ -0,0 +1,231 @@
> +#include <linux/string.h>
> +#include <linux/printk.h>
> +#include <linux/errno.h>
> +#include <linux/slab.h>
> +#include <linux/kexec.h>
> +#include <linux/kernel.h>
> +#include <linux/mm.h>
> +
> +#include <asm/bootparam.h>
> +#include <asm/setup.h>
> +
> +#ifdef CONFIG_X86_64
> +
> +struct elf_x86_64_data {
> +	/*
> +	 * Temporary buffer to hold bootparams buffer. This should be
> +	 * freed once the bootparam segment has been loaded.
> +	 */
> +	void *bootparams_buf;
> +};
> +
> +int elf_x86_64_probe(const char *buf, unsigned long len)
> +{
> +	int ret = -ENOEXEC;
> +	Elf_Ehdr *ehdr;
> +
> +	if (len < sizeof(Elf_Ehdr)) {
> +		pr_debug("File is too short to be an ELF executable.\n");
> +		return ret;
> +	}
> +
> +	ehdr = (Elf_Ehdr *)buf;
> +
> +	if (memcmp(ehdr->e_ident, ELFMAG, SELFMAG) != 0
> +	    || ehdr->e_type != ET_EXEC || !elf_check_arch(ehdr)
> +	    || ehdr->e_phentsize != sizeof(Elf_Phdr))
> +		return -ENOEXEC;
> +
> +	if (ehdr->e_phoff >= len
> +	    || (ehdr->e_phnum * sizeof(Elf_Phdr) > len - ehdr->e_phoff))
> +		return -ENOEXEC;
> +
> +        /* I've got a bzImage */
> +	pr_debug("It's an elf_x86_64 image.\n");
> +	ret = 0;
> +
> +	return ret;

I think you can drop 'ret' here and return the error vals directly.

> +}
> +
> +static int elf_exec_load(struct kimage *image, char *kernel)
> +{
> +	Elf_Ehdr *ehdr;
> +	Elf_Phdr *phdrs;
> +	int i, ret;
> +	size_t filesz;
> +	char *buffer;
> +
> +	ehdr = (Elf_Ehdr *)kernel;
> +	phdrs = (void *)ehdr + ehdr->e_phoff;
> +
> +	for (i = 0; i < ehdr->e_phnum; i++) {
> +		if (phdrs[i].p_type != PT_LOAD)
> +			continue;

newline

> +		filesz = phdrs[i].p_filesz;
> +		if (filesz > phdrs[i].p_memsz)
> +			filesz = phdrs[i].p_memsz;
> +
> +		buffer = (char *)ehdr + phdrs[i].p_offset;
> +		ret = kexec_add_segment(image, buffer, filesz, phdrs[i].p_memsz,
> +						phdrs[i].p_paddr);
> +		if (ret)
> +			break;
> +	}
> +
> +	return ret;
> +}
> +
> +/* Fill in fields which are usually present in bzImage */
> +static int init_linux_parameters(struct boot_params *params)
> +{
> +	/*
> +	 * FIXME: It is odd that the information which comes from kernel
> +	 * has to be faked by loading kernel. I guess it is limitation of
> +	 * ELF format. Right now keeping it same as kexec-tools
> +	 * implementation. But this most likely needs fixing.
> +	 */
> +	memcpy(&params->hdr.header, "HdrS", 4);
> +	params->hdr.version = 0x0206;
> +	params->hdr.initrd_addr_max = 0x37FFFFFF;
> +	params->hdr.cmdline_size = 2048;
> +	return 0;
> +}

Why a separate function? Its body is small enough to be merged into
elf_x86_64_load.

> +
> +void *elf_x86_64_load(struct kimage *image, char *kernel,
> +		unsigned long kernel_len,
> +		char *initrd, unsigned long initrd_len,
> +		char *cmdline, unsigned long cmdline_len)
> +{

Btw, this functionality below looks very similar to the one in
bzImage64_load(). Can we share some of it?

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

WARNING: multiple messages have this Message-ID (diff)
From: Borislav Petkov <bp@alien8.de>
To: Vivek Goyal <vgoyal@redhat.com>
Cc: linux-kernel@vger.kernel.org, kexec@lists.infradead.org,
	ebiederm@xmission.com, hpa@zytor.com, mjg59@srcf.ucam.org,
	greg@kroah.com, jkosina@suse.cz
Subject: Re: [PATCH 10/11] kexec: Support for loading ELF x86_64 images
Date: Fri, 28 Feb 2014 15:58:32 +0100	[thread overview]
Message-ID: <20140228145832.GF4326@pd.tnic> (raw)
In-Reply-To: <1390849071-21989-11-git-send-email-vgoyal@redhat.com>

On Mon, Jan 27, 2014 at 01:57:50PM -0500, Vivek Goyal wrote:
> This patch provides support for kexec for loading ELF x86_64 images. I have
> tested it with loading vmlinux and it worked.

Can you please enlighten me what the use case for ELF kernel images is? bzImage
I understand but what produces ELF images?

I see that kexec_file_load() can receive ELF segments too but why are we
doing that?

> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> ---
>  arch/x86/include/asm/kexec-elf.h   |  11 ++
>  arch/x86/kernel/Makefile           |   1 +
>  arch/x86/kernel/kexec-elf.c        | 231 +++++++++++++++++++++++++++++++++++++
>  arch/x86/kernel/machine_kexec_64.c |   2 +
>  4 files changed, 245 insertions(+)
>  create mode 100644 arch/x86/include/asm/kexec-elf.h
>  create mode 100644 arch/x86/kernel/kexec-elf.c
> 
> diff --git a/arch/x86/include/asm/kexec-elf.h b/arch/x86/include/asm/kexec-elf.h
> new file mode 100644
> index 0000000..afef382
> --- /dev/null
> +++ b/arch/x86/include/asm/kexec-elf.h
> @@ -0,0 +1,11 @@
> +#ifndef _ASM_KEXEC_ELF_H
> +#define _ASM_KEXEC_ELF_H
> +
> +extern int elf_x86_64_probe(const char *buf, unsigned long len);
> +extern void *elf_x86_64_load(struct kimage *image, char *kernel,
> +		unsigned long kernel_len, char *initrd,
> +		unsigned long initrd_len, char *cmdline,
> +		unsigned long cmdline_len);
> +extern int elf_x86_64_cleanup(struct kimage *image);
> +
> +#endif  /* _ASM_KEXEC_ELF_H */
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index fa9981d..2d77de7 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -71,6 +71,7 @@ obj-$(CONFIG_KEXEC)		+= machine_kexec.o
>  obj-$(CONFIG_KEXEC)		+= machine_kexec_$(BITS).o
>  obj-$(CONFIG_KEXEC)		+= relocate_kernel_$(BITS).o crash.o
>  obj-$(CONFIG_KEXEC)		+= kexec-bzimage.o
> +obj-$(CONFIG_KEXEC)		+= kexec-elf.o

It looks like kexec could slowly grow its own dir now:

arch/x86/kexec/

or so.

>  obj-$(CONFIG_CRASH_DUMP)	+= crash_dump_$(BITS).o
>  obj-y				+= kprobes/
>  obj-$(CONFIG_MODULES)		+= module.o
> diff --git a/arch/x86/kernel/kexec-elf.c b/arch/x86/kernel/kexec-elf.c
> new file mode 100644
> index 0000000..ff1017c
> --- /dev/null
> +++ b/arch/x86/kernel/kexec-elf.c
> @@ -0,0 +1,231 @@
> +#include <linux/string.h>
> +#include <linux/printk.h>
> +#include <linux/errno.h>
> +#include <linux/slab.h>
> +#include <linux/kexec.h>
> +#include <linux/kernel.h>
> +#include <linux/mm.h>
> +
> +#include <asm/bootparam.h>
> +#include <asm/setup.h>
> +
> +#ifdef CONFIG_X86_64
> +
> +struct elf_x86_64_data {
> +	/*
> +	 * Temporary buffer to hold bootparams buffer. This should be
> +	 * freed once the bootparam segment has been loaded.
> +	 */
> +	void *bootparams_buf;
> +};
> +
> +int elf_x86_64_probe(const char *buf, unsigned long len)
> +{
> +	int ret = -ENOEXEC;
> +	Elf_Ehdr *ehdr;
> +
> +	if (len < sizeof(Elf_Ehdr)) {
> +		pr_debug("File is too short to be an ELF executable.\n");
> +		return ret;
> +	}
> +
> +	ehdr = (Elf_Ehdr *)buf;
> +
> +	if (memcmp(ehdr->e_ident, ELFMAG, SELFMAG) != 0
> +	    || ehdr->e_type != ET_EXEC || !elf_check_arch(ehdr)
> +	    || ehdr->e_phentsize != sizeof(Elf_Phdr))
> +		return -ENOEXEC;
> +
> +	if (ehdr->e_phoff >= len
> +	    || (ehdr->e_phnum * sizeof(Elf_Phdr) > len - ehdr->e_phoff))
> +		return -ENOEXEC;
> +
> +        /* I've got a bzImage */
> +	pr_debug("It's an elf_x86_64 image.\n");
> +	ret = 0;
> +
> +	return ret;

I think you can drop 'ret' here and return the error vals directly.

> +}
> +
> +static int elf_exec_load(struct kimage *image, char *kernel)
> +{
> +	Elf_Ehdr *ehdr;
> +	Elf_Phdr *phdrs;
> +	int i, ret;
> +	size_t filesz;
> +	char *buffer;
> +
> +	ehdr = (Elf_Ehdr *)kernel;
> +	phdrs = (void *)ehdr + ehdr->e_phoff;
> +
> +	for (i = 0; i < ehdr->e_phnum; i++) {
> +		if (phdrs[i].p_type != PT_LOAD)
> +			continue;

newline

> +		filesz = phdrs[i].p_filesz;
> +		if (filesz > phdrs[i].p_memsz)
> +			filesz = phdrs[i].p_memsz;
> +
> +		buffer = (char *)ehdr + phdrs[i].p_offset;
> +		ret = kexec_add_segment(image, buffer, filesz, phdrs[i].p_memsz,
> +						phdrs[i].p_paddr);
> +		if (ret)
> +			break;
> +	}
> +
> +	return ret;
> +}
> +
> +/* Fill in fields which are usually present in bzImage */
> +static int init_linux_parameters(struct boot_params *params)
> +{
> +	/*
> +	 * FIXME: It is odd that the information which comes from kernel
> +	 * has to be faked by loading kernel. I guess it is limitation of
> +	 * ELF format. Right now keeping it same as kexec-tools
> +	 * implementation. But this most likely needs fixing.
> +	 */
> +	memcpy(&params->hdr.header, "HdrS", 4);
> +	params->hdr.version = 0x0206;
> +	params->hdr.initrd_addr_max = 0x37FFFFFF;
> +	params->hdr.cmdline_size = 2048;
> +	return 0;
> +}

Why a separate function? Its body is small enough to be merged into
elf_x86_64_load.

> +
> +void *elf_x86_64_load(struct kimage *image, char *kernel,
> +		unsigned long kernel_len,
> +		char *initrd, unsigned long initrd_len,
> +		char *cmdline, unsigned long cmdline_len)
> +{

Btw, this functionality below looks very similar to the one in
bzImage64_load(). Can we share some of it?

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

  reply	other threads:[~2014-02-28 14:59 UTC|newest]

Thread overview: 106+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-27 18:57 [RFC PATCH 00/11][V2] kexec: A new system call to allow in kernel loading Vivek Goyal
2014-01-27 18:57 ` Vivek Goyal
2014-01-27 18:57 ` [PATCH 01/11] kexec: Move segment verification code in a separate function Vivek Goyal
2014-01-27 18:57   ` Vivek Goyal
2014-01-27 18:57 ` [PATCH 02/11] resource: Provide new functions to walk through resources Vivek Goyal
2014-01-27 18:57   ` Vivek Goyal
2014-01-27 18:57 ` [PATCH 03/11] bin2c: Move bin2c in scripts/basic Vivek Goyal
2014-01-27 18:57   ` Vivek Goyal
2014-01-27 21:12   ` Michal Marek
2014-01-27 21:12     ` Michal Marek
2014-01-27 21:18     ` Vivek Goyal
2014-01-27 21:18       ` Vivek Goyal
2014-01-27 21:54       ` Michal Marek
2014-01-27 21:54         ` Michal Marek
2014-01-27 18:57 ` [PATCH 04/11] kernel: Build bin2c based on config option CONFIG_BUILD_BIN2C Vivek Goyal
2014-01-27 18:57   ` Vivek Goyal
2014-01-27 18:57 ` [PATCH 05/11] kexec: Make kexec_segment user buffer pointer a union Vivek Goyal
2014-01-27 18:57   ` Vivek Goyal
2014-01-27 18:57 ` [PATCH 06/11] kexec: A new system call, kexec_file_load, for in kernel kexec Vivek Goyal
2014-01-27 18:57   ` Vivek Goyal
2014-02-21 14:59   ` Borislav Petkov
2014-02-21 14:59     ` Borislav Petkov
2014-02-24 16:41     ` Vivek Goyal
2014-02-24 16:41       ` Vivek Goyal
2014-02-25 19:35       ` Petr Tesarik
2014-02-25 19:35         ` Petr Tesarik
2014-02-25 21:47         ` Borislav Petkov
2014-02-25 21:47           ` Borislav Petkov
2014-02-26 15:37       ` Borislav Petkov
2014-02-26 15:37         ` Borislav Petkov
2014-02-26 15:46         ` Vivek Goyal
2014-02-26 15:46           ` Vivek Goyal
2014-01-27 18:57 ` [PATCH 07/11] kexec: Create a relocatable object called purgatory Vivek Goyal
2014-01-27 18:57   ` Vivek Goyal
2014-02-24 19:08   ` H. Peter Anvin
2014-02-24 19:08     ` H. Peter Anvin
2014-02-25 16:43     ` Vivek Goyal
2014-02-25 16:43       ` Vivek Goyal
2014-02-25 16:55       ` H. Peter Anvin
2014-02-25 16:55         ` H. Peter Anvin
2014-02-25 18:20         ` Vivek Goyal
2014-02-25 18:20           ` Vivek Goyal
2014-02-25 21:09           ` H. Peter Anvin
2014-02-25 21:09             ` H. Peter Anvin
2014-02-26 14:52             ` Vivek Goyal
2014-02-26 14:52               ` Vivek Goyal
2014-02-26 16:00   ` Borislav Petkov
2014-02-26 16:00     ` Borislav Petkov
2014-02-26 16:32     ` Vivek Goyal
2014-02-26 16:32       ` Vivek Goyal
2014-02-27 15:44       ` Borislav Petkov
2014-02-27 15:44         ` Borislav Petkov
2014-01-27 18:57 ` [PATCH 08/11] kexec-bzImage: Support for loading bzImage using 64bit entry Vivek Goyal
2014-01-27 18:57   ` Vivek Goyal
2014-02-25 18:38   ` H. Peter Anvin
2014-02-25 18:38     ` H. Peter Anvin
2014-02-25 18:43     ` Vivek Goyal
2014-02-25 18:43       ` Vivek Goyal
2014-02-27 21:36   ` Borislav Petkov
2014-02-27 21:36     ` Borislav Petkov
2014-02-28 16:31     ` Vivek Goyal
2014-02-28 16:31       ` Vivek Goyal
2014-03-05 16:37       ` Borislav Petkov
2014-03-05 16:37         ` Borislav Petkov
2014-03-05 16:40         ` H. Peter Anvin
2014-03-05 16:40           ` H. Peter Anvin
2014-03-05 18:40         ` Vivek Goyal
2014-03-05 18:40           ` Vivek Goyal
2014-03-05 19:47           ` Borislav Petkov
2014-03-05 19:47             ` Borislav Petkov
2014-01-27 18:57 ` [PATCH 09/11] kexec: Provide a function to add a segment at fixed address Vivek Goyal
2014-01-27 18:57   ` Vivek Goyal
2014-02-27 21:52   ` Borislav Petkov
2014-02-27 21:52     ` Borislav Petkov
2014-02-28 16:56     ` Vivek Goyal
2014-02-28 16:56       ` Vivek Goyal
2014-03-10 10:01       ` Borislav Petkov
2014-03-10 10:01         ` Borislav Petkov
2014-03-10 15:35         ` Vivek Goyal
2014-03-10 15:35           ` Vivek Goyal
2014-01-27 18:57 ` [PATCH 10/11] kexec: Support for loading ELF x86_64 images Vivek Goyal
2014-01-27 18:57   ` Vivek Goyal
2014-02-28 14:58   ` Borislav Petkov [this message]
2014-02-28 14:58     ` Borislav Petkov
2014-02-28 17:11     ` Vivek Goyal
2014-02-28 17:11       ` Vivek Goyal
2014-03-07 17:12       ` Borislav Petkov
2014-03-07 17:12         ` Borislav Petkov
2014-03-07 18:39         ` Borislav Petkov
2014-03-07 18:39           ` Borislav Petkov
2014-03-10 14:42           ` Vivek Goyal
2014-03-10 14:42             ` Vivek Goyal
2014-03-12 16:19             ` Borislav Petkov
2014-03-12 16:19               ` Borislav Petkov
2014-03-12 17:24               ` Vivek Goyal
2014-03-12 17:24                 ` Vivek Goyal
2014-01-27 18:57 ` [PATCH 11/11] kexec: Support for Kexec on panic using new system call Vivek Goyal
2014-01-27 18:57   ` Vivek Goyal
2014-02-28 17:28   ` Borislav Petkov
2014-02-28 17:28     ` Borislav Petkov
2014-02-28 21:06     ` Vivek Goyal
2014-02-28 21:06       ` Vivek Goyal
2014-05-26  8:25 ` [RFC PATCH 00/11][V2] kexec: A new system call to allow in kernel loading Borislav Petkov
2014-05-26  8:25   ` Borislav Petkov
2014-05-27 12:34   ` Vivek Goyal
2014-05-27 12:34     ` Vivek Goyal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140228145832.GF4326@pd.tnic \
    --to=bp@alien8.de \
    --cc=ebiederm@xmission.com \
    --cc=greg@kroah.com \
    --cc=hpa@zytor.com \
    --cc=jkosina@suse.cz \
    --cc=kexec@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mjg59@srcf.ucam.org \
    --cc=vgoyal@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.