On Wed, Sep 20, 2006 at 12:44:41PM -0400, Vivek Goyal wrote: > On Tue, Sep 19, 2006 at 10:46:33AM +0900, Horms wrote:
[snip] > > > + /* Don't perform checks to make sure stated phdrs and shdrs are > > > + * actually present in the core file. It is not practical > > > + * to read the GB size file into a user space buffer, Given the > > > + * fact that we don't use any info from that. > > > + */ > > > > That is true, but it is might be practical to memmap it. > > Have you given some thought to whether that is worthwile or not? > > I tried it but /proc/kcore does not support mmapping it. Otherwise > its a good idea. Ok, I was wondering about that as I wrote my original comment. > > > + ignore_elf_len_check = 1; > > > > Should ignore_elf_len_check be turned off again at some point, > > or alternatively changed into a parameter rather than a global? > > Now I am turning it off. > > Its a global for keeping the code changes less. If I make it a parameter > then I have to change whole lot of functions and code across arches due > to interdependencies. So for the time being I have kept it a separate > global. But if you think that it is absolute no-no programming practice, > then I will write a separate patch to make it a function parameter. That is a bit tricky isn't it. Personally I am more in favour of parameters over globals. But I understand the volume of change issue. I think what you have is ok for now. But for interests sake I made a patch to implement the parameter approach. It is against the updated patch you sent earlier today. It could be improved by pasing a general flags parameter, rather than a specific ignore_len_check parameter. The patch is below. [snip] > > I'm not sure that I understand the alingment that the above > > if statment is adding, but I guess its fine :-) > > Actually /proc/kcore reports the vaddr of symbol _stext > and not actually the starting kernel virtual address in the program > header created for kernel text. Generally there is some code before symbol > _stext. Anyway I load the kernel at 2MB aligned boundary so I forced this > alignment to make sure a neat separate program header is created. Otherwise > in /proc/vmcore we will end up creating an header starting at and odd > address, something like 0xfffff....1f1. Ok, thanks for clarifying that. [snip] -- Horms H: http://www.vergenet.net/~horms/ W: http://www.valinux.co.jp/en/ kexec-tool: non-global ignore_elf_len_check This is a very noisy patch that makes ignore_elf_len_check non-global Signed-Off-By: Simon Horman <[EMAIL PROTECTED]> --- kexec/arch/i386/kexec-elf-x86.c | 4 +-- kexec/arch/i386/kexec-multiboot-x86.c | 2 - kexec/kexec-elf-core.c | 2 - kexec/kexec-elf-exec.c | 9 ++++--- kexec/kexec-elf-rel.c | 2 - kexec/kexec-elf.c | 28 ++++++++---------------- kexec/kexec-elf.h | 8 ++++-- 7 files changed, 25 insertions(+), 30 deletions(-) Index: kexec-tools-build-fix+vivek_kcore/kexec/arch/i386/kexec-elf-x86.c =================================================================== --- kexec-tools-build-fix+vivek_kcore.orig/kexec/arch/i386/kexec-elf-x86.c 2006-09-21 17:08:58.000000000 +0900 +++ kexec-tools-build-fix+vivek_kcore/kexec/arch/i386/kexec-elf-x86.c 2006-09-21 17:09:19.000000000 +0900 @@ -47,7 +47,7 @@ struct mem_ehdr ehdr; int result; - result = build_elf_exec_info(buf, len, &ehdr); + result = build_elf_exec_info(buf, len, &ehdr, 0); if (result < 0) { if (probe_debug) { fprintf(stderr, "Not an ELF executable\n"); @@ -177,7 +177,7 @@ } /* Load the ELF executable */ - elf_exec_build_load(info, &ehdr, buf, len); + elf_exec_build_load(info, &ehdr, buf, len, 0); entry = ehdr.e_entry; max_addr = elf_max_addr(&ehdr); Index: kexec-tools-build-fix+vivek_kcore/kexec/arch/i386/kexec-multiboot-x86.c =================================================================== --- kexec-tools-build-fix+vivek_kcore.orig/kexec/arch/i386/kexec-multiboot-x86.c 2006-09-21 17:09:34.000000000 +0900 +++ kexec-tools-build-fix+vivek_kcore/kexec/arch/i386/kexec-multiboot-x86.c 2006-09-21 17:09:50.000000000 +0900 @@ -210,7 +210,7 @@ /* Load the ELF executable */ - elf_exec_build_load(info, &ehdr, buf, len); + elf_exec_build_load(info, &ehdr, buf, len, 0); /* Load the setup code */ elf_rel_build_load(info, &info->rhdr, purgatory, purgatory_size, 0, ULONG_MAX, 1); Index: kexec-tools-build-fix+vivek_kcore/kexec/kexec-elf.c =================================================================== --- kexec-tools-build-fix+vivek_kcore.orig/kexec/kexec-elf.c 2006-09-21 16:55:50.000000000 +0900 +++ kexec-tools-build-fix+vivek_kcore/kexec/kexec-elf.c 2006-09-21 16:58:32.000000000 +0900 @@ -11,17 +11,6 @@ static const int probe_debug = 0; -/* This parameter can be set to force ELF phdr and shdr building routines - * to skip the check on file length which make sure that data pointed by - * phdrs and shdrs is actually present in the file. This has been introduced - * to handle the corner case of reading /proc/kcore. /proc/kcore can be - * of huge size (GB, TB), depending on RAM size. It does not make sense - * to read that whole core file just to extract some info from kcore ELF - * headers. - * This is hackish. Please suggest a better way to handle it. - */ -int ignore_elf_len_check = 0; - uint16_t elf16_to_cpu(const struct mem_ehdr *ehdr, uint16_t value) { if (ehdr->ei_data == ELFDATA2LSB) { @@ -379,7 +368,8 @@ return 0; } -static int build_mem_phdrs(const char *buf, off_t len, struct mem_ehdr *ehdr) +static int build_mem_phdrs(const char *buf, off_t len, struct mem_ehdr *ehdr, + int ignore_len_check) { size_t phdr_size, mem_phdr_size; int i; @@ -431,7 +421,7 @@ * they are safe to use. */ phdr = &ehdr->e_phdr[i]; - if (!ignore_elf_len_check && + if (!ignore_len_check && (phdr->p_offset + phdr->p_filesz) > len) { /* The segment does not fit in the buffer */ @@ -592,7 +582,8 @@ return 0; } -static int build_mem_shdrs(const char *buf, off_t len, struct mem_ehdr *ehdr) +static int build_mem_shdrs(const char *buf, off_t len, struct mem_ehdr *ehdr, + int ignore_len_check) { size_t shdr_size, mem_shdr_size; int i; @@ -642,7 +633,7 @@ * they are safe to use. */ shdr = &ehdr->e_shdr[i]; - if (!ignore_elf_len_check && (shdr->sh_type != SHT_NOBITS) + if (!ignore_len_check && (shdr->sh_type != SHT_NOBITS) && (shdr->sh_offset + shdr->sh_size) > len) { /* The section does not fit in the buffer */ if (probe_debug) @@ -740,7 +731,8 @@ memset(ehdr, 0, sizeof(*ehdr)); } -int build_elf_info(const char *buf, off_t len, struct mem_ehdr *ehdr) +int build_elf_info(const char *buf, off_t len, struct mem_ehdr *ehdr, + int ignore_len_check) { int result; result = build_mem_ehdr(buf, len, ehdr); @@ -748,14 +740,14 @@ return result; } if ((ehdr->e_phoff > 0) && (ehdr->e_phnum > 0)) { - result = build_mem_phdrs(buf, len, ehdr); + result = build_mem_phdrs(buf, len, ehdr, ignore_len_check); if (result < 0) { free_elf_info(ehdr); return result; } } if ((ehdr->e_shoff > 0) && (ehdr->e_shnum > 0)) { - result = build_mem_shdrs(buf, len, ehdr); + result = build_mem_shdrs(buf, len, ehdr, ignore_len_check); if (result < 0) { free_elf_info(ehdr); return result; Index: kexec-tools-build-fix+vivek_kcore/kexec/kexec-elf-core.c =================================================================== --- kexec-tools-build-fix+vivek_kcore.orig/kexec/kexec-elf-core.c 2006-09-21 17:05:06.000000000 +0900 +++ kexec-tools-build-fix+vivek_kcore/kexec/kexec-elf-core.c 2006-09-21 17:06:11.000000000 +0900 @@ -9,7 +9,7 @@ int build_elf_core_info(const char *buf, off_t len, struct mem_ehdr *ehdr) { int result; - result = build_elf_info(buf, len, ehdr); + result = build_elf_info(buf, len, ehdr, 1); if (result < 0) { return result; } Index: kexec-tools-build-fix+vivek_kcore/kexec/kexec-elf-exec.c =================================================================== --- kexec-tools-build-fix+vivek_kcore.orig/kexec/kexec-elf-exec.c 2006-09-21 16:59:57.000000000 +0900 +++ kexec-tools-build-fix+vivek_kcore/kexec/kexec-elf-exec.c 2006-09-21 17:04:33.000000000 +0900 @@ -11,11 +11,12 @@ static const int probe_debug = 0; -int build_elf_exec_info(const char *buf, off_t len, struct mem_ehdr *ehdr) +int build_elf_exec_info(const char *buf, off_t len, struct mem_ehdr *ehdr, + int ignore_len_check) { struct mem_phdr *phdr, *end_phdr; int result; - result = build_elf_info(buf, len, ehdr); + result = build_elf_info(buf, len, ehdr, ignore_len_check); if (result < 0) { return result; } @@ -136,11 +137,11 @@ } void elf_exec_build_load(struct kexec_info *info, struct mem_ehdr *ehdr, - const char *buf, off_t len) + const char *buf, off_t len, int ignore_len_check) { int result; /* Parse the Elf file */ - result = build_elf_exec_info(buf, len, ehdr); + result = build_elf_exec_info(buf, len, ehdr, ignore_len_check); if (result < 0) { die("ELF exec parse failed\n"); } Index: kexec-tools-build-fix+vivek_kcore/kexec/kexec-elf.h =================================================================== --- kexec-tools-build-fix+vivek_kcore.orig/kexec/kexec-elf.h 2006-09-21 16:59:11.000000000 +0900 +++ kexec-tools-build-fix+vivek_kcore/kexec/kexec-elf.h 2006-09-21 17:04:51.000000000 +0900 @@ -85,8 +85,10 @@ extern void free_elf_info(struct mem_ehdr *ehdr); -extern int build_elf_info(const char *buf, off_t len, struct mem_ehdr *ehdr); -extern int build_elf_exec_info(const char *buf, off_t len, struct mem_ehdr *ehdr); +extern int build_elf_info(const char *buf, off_t len, struct mem_ehdr *ehdr, + int ignore_len_check); +extern int build_elf_exec_info(const char *buf, off_t len, + struct mem_ehdr *ehdr, int ignore_len_check); extern int build_elf_rel_info(const char *buf, off_t len, struct mem_ehdr *ehdr); extern int build_elf_core_info(const char *buf, off_t len, struct mem_ehdr *ehdr); @@ -95,7 +97,7 @@ unsigned long min, unsigned long max, int end); extern void elf_exec_build_load(struct kexec_info *info, struct mem_ehdr *ehdr, - const char *buf, off_t len); + const char *buf, off_t len, int ignore_len_check); extern void elf_rel_build_load(struct kexec_info *info, struct mem_ehdr *ehdr, const char *buf, off_t len, unsigned long min, unsigned long max, int end); Index: kexec-tools-build-fix+vivek_kcore/kexec/kexec-elf-rel.c =================================================================== --- kexec-tools-build-fix+vivek_kcore.orig/kexec/kexec-elf-rel.c 2006-09-21 17:07:19.000000000 +0900 +++ kexec-tools-build-fix+vivek_kcore/kexec/kexec-elf-rel.c 2006-09-21 17:08:29.000000000 +0900 @@ -138,7 +138,7 @@ int build_elf_rel_info(const char *buf, off_t len, struct mem_ehdr *ehdr) { int result; - result = build_elf_info(buf, len, ehdr); + result = build_elf_info(buf, len, ehdr, 0); if (result < 0) { return result; } _______________________________________________ fastboot mailing list [email protected] https://lists.osdl.org/mailman/listinfo/fastboot
