* [PATCH 1/4] elf: don't be afraid of overflow
@ 2019-02-04 20:27 Alexey Dobriyan
2019-02-04 20:28 ` [PATCH 2/4] elf: use list_for_each_entry() Alexey Dobriyan
2019-03-11 11:04 ` [PATCH 1/4] elf: don't be afraid of overflow Pavel Machek
0 siblings, 2 replies; 9+ messages in thread
From: Alexey Dobriyan @ 2019-02-04 20:27 UTC (permalink / raw)
To: akpm; +Cc: linux-kernel
Number of ELF program headers is 16-bit by spec, so total size
comfortably fits into "unsigned int".
Space savings: 7 bytes!
add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-7 (-7)
Function old new delta
load_elf_phdrs 137 130 -7
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
fs/binfmt_elf.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -418,8 +418,9 @@ static struct elf_phdr *load_elf_phdrs(struct elfhdr *elf_ex,
struct file *elf_file)
{
struct elf_phdr *elf_phdata = NULL;
- int retval, size, err = -1;
+ int retval, err = -1;
loff_t pos = elf_ex->e_phoff;
+ unsigned int size;
/*
* If the size of this structure has changed, then punt, since
@@ -429,13 +430,9 @@ static struct elf_phdr *load_elf_phdrs(struct elfhdr *elf_ex,
goto out;
/* Sanity check the number of program headers... */
- if (elf_ex->e_phnum < 1 ||
- elf_ex->e_phnum > 65536U / sizeof(struct elf_phdr))
- goto out;
-
/* ...and their total size. */
size = sizeof(struct elf_phdr) * elf_ex->e_phnum;
- if (size > ELF_MIN_ALIGN)
+ if (size == 0 || size > 65536 || size > ELF_MIN_ALIGN)
goto out;
elf_phdata = kmalloc(size, GFP_KERNEL);
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 2/4] elf: use list_for_each_entry() 2019-02-04 20:27 [PATCH 1/4] elf: don't be afraid of overflow Alexey Dobriyan @ 2019-02-04 20:28 ` Alexey Dobriyan 2019-02-04 20:28 ` [PATCH 3/4] elf: spread const a little Alexey Dobriyan ` (2 more replies) 2019-03-11 11:04 ` [PATCH 1/4] elf: don't be afraid of overflow Pavel Machek 1 sibling, 3 replies; 9+ messages in thread From: Alexey Dobriyan @ 2019-02-04 20:28 UTC (permalink / raw) To: akpm; +Cc: linux-kernel Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com> --- fs/binfmt_elf.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -2030,7 +2030,6 @@ static int fill_note_info(struct elfhdr *elf, int phdrs, struct elf_note_info *info, const kernel_siginfo_t *siginfo, struct pt_regs *regs) { - struct list_head *t; struct core_thread *ct; struct elf_thread_status *ets; @@ -2047,10 +2046,9 @@ static int fill_note_info(struct elfhdr *elf, int phdrs, list_add(&ets->list, &info->thread_list); } - list_for_each(t, &info->thread_list) { + list_for_each_entry(ets, &info->thread_list, list) { int sz; - ets = list_entry(t, struct elf_thread_status, list); sz = elf_dump_thread_status(siginfo->si_signo, ets); info->thread_status_size += sz; } @@ -2114,18 +2112,15 @@ static size_t get_note_info_size(struct elf_note_info *info) static int write_note_info(struct elf_note_info *info, struct coredump_params *cprm) { + struct elf_thread_status *ets; int i; - struct list_head *t; for (i = 0; i < info->numnote; i++) if (!writenote(info->notes + i, cprm)) return 0; /* write out the thread status notes section */ - list_for_each(t, &info->thread_list) { - struct elf_thread_status *tmp = - list_entry(t, struct elf_thread_status, list); - + list_for_each_entry(ets, &info->thread_list, list) { for (i = 0; i < tmp->num_notes; i++) if (!writenote(&tmp->notes[i], cprm)) return 0; ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/4] elf: spread const a little 2019-02-04 20:28 ` [PATCH 2/4] elf: use list_for_each_entry() Alexey Dobriyan @ 2019-02-04 20:28 ` Alexey Dobriyan 2019-02-04 20:33 ` [PATCH 4/4] elf: save allocation per exec Alexey Dobriyan 2019-02-12 15:49 ` [PATCH 2/4] elf: use list_for_each_entry() kbuild test robot 2019-02-12 16:21 ` kbuild test robot 2 siblings, 1 reply; 9+ messages in thread From: Alexey Dobriyan @ 2019-02-04 20:28 UTC (permalink / raw) To: akpm; +Cc: linux-kernel Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com> --- fs/binfmt_elf.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -57,8 +57,6 @@ #endif static int load_elf_binary(struct linux_binprm *bprm); -static unsigned long elf_map(struct file *, unsigned long, struct elf_phdr *, - int, int, unsigned long); #ifdef CONFIG_USELIB static int load_elf_library(struct file *); @@ -347,7 +345,7 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec, #ifndef elf_map static unsigned long elf_map(struct file *filep, unsigned long addr, - struct elf_phdr *eppnt, int prot, int type, + const struct elf_phdr *eppnt, int prot, int type, unsigned long total_size) { unsigned long map_addr; @@ -387,7 +385,7 @@ static unsigned long elf_map(struct file *filep, unsigned long addr, #endif /* !elf_map */ -static unsigned long total_mapping_size(struct elf_phdr *cmds, int nr) +static unsigned long total_mapping_size(const struct elf_phdr *cmds, int nr) { int i, first_idx = -1, last_idx = -1; @@ -414,7 +412,7 @@ static unsigned long total_mapping_size(struct elf_phdr *cmds, int nr) * header pointed to by elf_ex, into a newly allocated array. The caller is * responsible for freeing the allocated data. Returns an ERR_PTR upon failure. */ -static struct elf_phdr *load_elf_phdrs(struct elfhdr *elf_ex, +static struct elf_phdr *load_elf_phdrs(const struct elfhdr *elf_ex, struct file *elf_file) { struct elf_phdr *elf_phdata = NULL; ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/4] elf: save allocation per exec 2019-02-04 20:28 ` [PATCH 3/4] elf: spread const a little Alexey Dobriyan @ 2019-02-04 20:33 ` Alexey Dobriyan 0 siblings, 0 replies; 9+ messages in thread From: Alexey Dobriyan @ 2019-02-04 20:33 UTC (permalink / raw) To: akpm; +Cc: linux-kernel 2 ELF header aren't that large to allocate them on heap. However, code is bloated by 1.5 KB (!!!) with gcc 8.2 and stack size grows by ~340 bytes. The problem is that 2 elf headers are only 128 bytes. I'm shocked and the patch should not be applied probably. Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com> --- fs/binfmt_elf.c | 78 ++++++++++++++++++++++++-------------------------------- 1 file changed, 34 insertions(+), 44 deletions(-) --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -699,37 +699,29 @@ static int load_elf_binary(struct linux_binprm *bprm) unsigned long reloc_func_desc __maybe_unused = 0; int executable_stack = EXSTACK_DEFAULT; struct pt_regs *regs = current_pt_regs(); - struct { - struct elfhdr elf_ex; - struct elfhdr interp_elf_ex; - } *loc; + struct elfhdr elf_ex; + struct elfhdr interp_elf_ex; struct arch_elf_state arch_state = INIT_ARCH_ELF_STATE; loff_t pos; - loc = kmalloc(sizeof(*loc), GFP_KERNEL); - if (!loc) { - retval = -ENOMEM; - goto out_ret; - } - /* Get the exec-header */ - loc->elf_ex = *((struct elfhdr *)bprm->buf); + elf_ex = *(struct elfhdr *)bprm->buf; retval = -ENOEXEC; /* First of all, some simple consistency checks */ - if (memcmp(loc->elf_ex.e_ident, ELFMAG, SELFMAG) != 0) + if (memcmp(elf_ex.e_ident, ELFMAG, SELFMAG) != 0) goto out; - if (loc->elf_ex.e_type != ET_EXEC && loc->elf_ex.e_type != ET_DYN) + if (elf_ex.e_type != ET_EXEC && elf_ex.e_type != ET_DYN) goto out; - if (!elf_check_arch(&loc->elf_ex)) + if (!elf_check_arch(&elf_ex)) goto out; - if (elf_check_fdpic(&loc->elf_ex)) + if (elf_check_fdpic(&elf_ex)) goto out; if (!bprm->file->f_op->mmap) goto out; - elf_phdata = load_elf_phdrs(&loc->elf_ex, bprm->file); + elf_phdata = load_elf_phdrs(&elf_ex, bprm->file); if (!elf_phdata) goto out; @@ -742,7 +734,7 @@ static int load_elf_binary(struct linux_binprm *bprm) start_data = 0; end_data = 0; - for (i = 0; i < loc->elf_ex.e_phnum; i++) { + for (i = 0; i < elf_ex.e_phnum; i++) { if (elf_ppnt->p_type == PT_INTERP) { /* This is the program interpreter used for * shared libraries - for now assume that this @@ -786,9 +778,9 @@ static int load_elf_binary(struct linux_binprm *bprm) /* Get the exec headers */ pos = 0; - retval = kernel_read(interpreter, &loc->interp_elf_ex, - sizeof(loc->interp_elf_ex), &pos); - if (retval != sizeof(loc->interp_elf_ex)) { + retval = kernel_read(interpreter, &interp_elf_ex, + sizeof(interp_elf_ex), &pos); + if (retval != sizeof(interp_elf_ex)) { if (retval >= 0) retval = -EIO; goto out_free_dentry; @@ -800,7 +792,7 @@ static int load_elf_binary(struct linux_binprm *bprm) } elf_ppnt = elf_phdata; - for (i = 0; i < loc->elf_ex.e_phnum; i++, elf_ppnt++) + for (i = 0; i < elf_ex.e_phnum; i++, elf_ppnt++) switch (elf_ppnt->p_type) { case PT_GNU_STACK: if (elf_ppnt->p_flags & PF_X) @@ -810,7 +802,7 @@ static int load_elf_binary(struct linux_binprm *bprm) break; case PT_LOPROC ... PT_HIPROC: - retval = arch_elf_pt_proc(&loc->elf_ex, elf_ppnt, + retval = arch_elf_pt_proc(&elf_ex, elf_ppnt, bprm->file, false, &arch_state); if (retval) @@ -822,25 +814,25 @@ static int load_elf_binary(struct linux_binprm *bprm) if (elf_interpreter) { retval = -ELIBBAD; /* Not an ELF interpreter */ - if (memcmp(loc->interp_elf_ex.e_ident, ELFMAG, SELFMAG) != 0) + if (memcmp(interp_elf_ex.e_ident, ELFMAG, SELFMAG) != 0) goto out_free_dentry; /* Verify the interpreter has a valid arch */ - if (!elf_check_arch(&loc->interp_elf_ex) || - elf_check_fdpic(&loc->interp_elf_ex)) + if (!elf_check_arch(&interp_elf_ex) || + elf_check_fdpic(&interp_elf_ex)) goto out_free_dentry; /* Load the interpreter program headers */ - interp_elf_phdata = load_elf_phdrs(&loc->interp_elf_ex, + interp_elf_phdata = load_elf_phdrs(&interp_elf_ex, interpreter); if (!interp_elf_phdata) goto out_free_dentry; /* Pass PT_LOPROC..PT_HIPROC headers to arch code */ elf_ppnt = interp_elf_phdata; - for (i = 0; i < loc->interp_elf_ex.e_phnum; i++, elf_ppnt++) + for (i = 0; i < interp_elf_ex.e_phnum; i++, elf_ppnt++) switch (elf_ppnt->p_type) { case PT_LOPROC ... PT_HIPROC: - retval = arch_elf_pt_proc(&loc->interp_elf_ex, + retval = arch_elf_pt_proc(&interp_elf_ex, elf_ppnt, interpreter, true, &arch_state); if (retval) @@ -854,8 +846,8 @@ static int load_elf_binary(struct linux_binprm *bprm) * still possible to return an error to the code that invoked * the exec syscall. */ - retval = arch_check_elf(&loc->elf_ex, - !!interpreter, &loc->interp_elf_ex, + retval = arch_check_elf(&elf_ex, + !!interpreter, &interp_elf_ex, &arch_state); if (retval) goto out_free_dentry; @@ -867,8 +859,8 @@ static int load_elf_binary(struct linux_binprm *bprm) /* Do this immediately, since STACK_TOP as used in setup_arg_pages may depend on the personality. */ - SET_PERSONALITY2(loc->elf_ex, &arch_state); - if (elf_read_implies_exec(loc->elf_ex, executable_stack)) + SET_PERSONALITY2(elf_ex, &arch_state); + if (elf_read_implies_exec(elf_ex, executable_stack)) current->personality |= READ_IMPLIES_EXEC; if (!(current->personality & ADDR_NO_RANDOMIZE) && randomize_va_space) @@ -889,7 +881,7 @@ static int load_elf_binary(struct linux_binprm *bprm) /* Now we do a little grungy work by mmapping the ELF image into the correct location in memory. */ for(i = 0, elf_ppnt = elf_phdata; - i < loc->elf_ex.e_phnum; i++, elf_ppnt++) { + i < elf_ex.e_phnum; i++, elf_ppnt++) { int elf_prot = 0, elf_flags, elf_fixed = MAP_FIXED_NOREPLACE; unsigned long k, vaddr; unsigned long total_size = 0; @@ -945,9 +937,9 @@ static int load_elf_binary(struct linux_binprm *bprm) * If we are loading ET_EXEC or we have already performed * the ET_DYN load_addr calculations, proceed normally. */ - if (loc->elf_ex.e_type == ET_EXEC || load_addr_set) { + if (elf_ex.e_type == ET_EXEC || load_addr_set) { elf_flags |= elf_fixed; - } else if (loc->elf_ex.e_type == ET_DYN) { + } else if (elf_ex.e_type == ET_DYN) { /* * This logic is run once for the first LOAD Program * Header for ET_DYN binaries to calculate the @@ -996,7 +988,7 @@ static int load_elf_binary(struct linux_binprm *bprm) load_bias = ELF_PAGESTART(load_bias - vaddr); total_size = total_mapping_size(elf_phdata, - loc->elf_ex.e_phnum); + elf_ex.e_phnum); if (!total_size) { retval = -EINVAL; goto out_free_dentry; @@ -1014,7 +1006,7 @@ static int load_elf_binary(struct linux_binprm *bprm) if (!load_addr_set) { load_addr_set = 1; load_addr = (elf_ppnt->p_vaddr - elf_ppnt->p_offset); - if (loc->elf_ex.e_type == ET_DYN) { + if (elf_ex.e_type == ET_DYN) { load_bias += error - ELF_PAGESTART(load_bias + vaddr); load_addr += load_bias; @@ -1055,7 +1047,7 @@ static int load_elf_binary(struct linux_binprm *bprm) } } - loc->elf_ex.e_entry += load_bias; + elf_ex.e_entry += load_bias; elf_bss += load_bias; elf_brk += load_bias; start_code += load_bias; @@ -1079,7 +1071,7 @@ static int load_elf_binary(struct linux_binprm *bprm) if (elf_interpreter) { unsigned long interp_map_addr = 0; - elf_entry = load_elf_interp(&loc->interp_elf_ex, + elf_entry = load_elf_interp(&interp_elf_ex, interpreter, &interp_map_addr, load_bias, interp_elf_phdata); @@ -1089,7 +1081,7 @@ static int load_elf_binary(struct linux_binprm *bprm) * adjustment */ interp_load_addr = elf_entry; - elf_entry += loc->interp_elf_ex.e_entry; + elf_entry += interp_elf_ex.e_entry; } if (BAD_ADDR(elf_entry)) { retval = IS_ERR((void *)elf_entry) ? @@ -1102,7 +1094,7 @@ static int load_elf_binary(struct linux_binprm *bprm) fput(interpreter); kfree(elf_interpreter); } else { - elf_entry = loc->elf_ex.e_entry; + elf_entry = elf_ex.e_entry; if (BAD_ADDR(elf_entry)) { retval = -EINVAL; goto out_free_dentry; @@ -1120,7 +1112,7 @@ static int load_elf_binary(struct linux_binprm *bprm) goto out; #endif /* ARCH_HAS_SETUP_ADDITIONAL_PAGES */ - retval = create_elf_tables(bprm, &loc->elf_ex, + retval = create_elf_tables(bprm, &elf_ex, load_addr, interp_load_addr); if (retval < 0) goto out; @@ -1166,8 +1158,6 @@ static int load_elf_binary(struct linux_binprm *bprm) start_thread(regs, elf_entry, bprm->p); retval = 0; out: - kfree(loc); -out_ret: return retval; /* error cleanup */ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/4] elf: use list_for_each_entry() 2019-02-04 20:28 ` [PATCH 2/4] elf: use list_for_each_entry() Alexey Dobriyan 2019-02-04 20:28 ` [PATCH 3/4] elf: spread const a little Alexey Dobriyan @ 2019-02-12 15:49 ` kbuild test robot 2019-02-12 16:21 ` kbuild test robot 2 siblings, 0 replies; 9+ messages in thread From: kbuild test robot @ 2019-02-12 15:49 UTC (permalink / raw) To: Alexey Dobriyan; +Cc: kbuild-all, akpm, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2839 bytes --] Hi Alexey, I love your patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v5.0-rc4] [cannot apply to next-20190212] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Alexey-Dobriyan/elf-don-t-be-afraid-of-overflow/20190205-225931 config: nds32-defconfig (attached as .config) compiler: nds32le-linux-gcc (GCC) 6.4.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=6.4.0 make.cross ARCH=nds32 All errors (new ones prefixed by >>): fs/binfmt_elf.c: In function 'write_note_info': >> fs/binfmt_elf.c:2124:19: error: 'tmp' undeclared (first use in this function) for (i = 0; i < tmp->num_notes; i++) ^~~ fs/binfmt_elf.c:2124:19: note: each undeclared identifier is reported only once for each function it appears in vim +/tmp +2124 fs/binfmt_elf.c 3aba481f Roland McGrath 2008-01-30 2111 3aba481f Roland McGrath 2008-01-30 2112 static int write_note_info(struct elf_note_info *info, ecc8c772 Al Viro 2013-10-05 2113 struct coredump_params *cprm) 3aba481f Roland McGrath 2008-01-30 2114 { 1e0d184d Alexey Dobriyan 2019-02-04 2115 struct elf_thread_status *ets; 3aba481f Roland McGrath 2008-01-30 2116 int i; 3aba481f Roland McGrath 2008-01-30 2117 3aba481f Roland McGrath 2008-01-30 2118 for (i = 0; i < info->numnote; i++) ecc8c772 Al Viro 2013-10-05 2119 if (!writenote(info->notes + i, cprm)) 3aba481f Roland McGrath 2008-01-30 2120 return 0; 3aba481f Roland McGrath 2008-01-30 2121 3aba481f Roland McGrath 2008-01-30 2122 /* write out the thread status notes section */ 1e0d184d Alexey Dobriyan 2019-02-04 2123 list_for_each_entry(ets, &info->thread_list, list) { 3aba481f Roland McGrath 2008-01-30 @2124 for (i = 0; i < tmp->num_notes; i++) ecc8c772 Al Viro 2013-10-05 2125 if (!writenote(&tmp->notes[i], cprm)) 3aba481f Roland McGrath 2008-01-30 2126 return 0; 3aba481f Roland McGrath 2008-01-30 2127 } 3aba481f Roland McGrath 2008-01-30 2128 3aba481f Roland McGrath 2008-01-30 2129 return 1; 3aba481f Roland McGrath 2008-01-30 2130 } 3aba481f Roland McGrath 2008-01-30 2131 :::::: The code at line 2124 was first introduced by commit :::::: 3aba481fc94d83ff630d4b7cd2f7447010c4c6df elf core dump: notes reorg :::::: TO: Roland McGrath <roland@redhat.com> :::::: CC: Ingo Molnar <mingo@elte.hu> --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 9712 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/4] elf: use list_for_each_entry() 2019-02-04 20:28 ` [PATCH 2/4] elf: use list_for_each_entry() Alexey Dobriyan 2019-02-04 20:28 ` [PATCH 3/4] elf: spread const a little Alexey Dobriyan 2019-02-12 15:49 ` [PATCH 2/4] elf: use list_for_each_entry() kbuild test robot @ 2019-02-12 16:21 ` kbuild test robot 2 siblings, 0 replies; 9+ messages in thread From: kbuild test robot @ 2019-02-12 16:21 UTC (permalink / raw) To: Alexey Dobriyan; +Cc: kbuild-all, akpm, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2894 bytes --] Hi Alexey, I love your patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v5.0-rc4] [cannot apply to next-20190212] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Alexey-Dobriyan/elf-don-t-be-afraid-of-overflow/20190205-225931 config: microblaze-allmodconfig (attached as .config) compiler: microblaze-linux-gcc (GCC) 8.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=8.2.0 make.cross ARCH=microblaze All errors (new ones prefixed by >>): fs/binfmt_elf.c: In function 'write_note_info': >> fs/binfmt_elf.c:2124:19: error: 'tmp' undeclared (first use in this function); did you mean 'tm'? for (i = 0; i < tmp->num_notes; i++) ^~~ tm fs/binfmt_elf.c:2124:19: note: each undeclared identifier is reported only once for each function it appears in vim +2124 fs/binfmt_elf.c 3aba481f Roland McGrath 2008-01-30 2111 3aba481f Roland McGrath 2008-01-30 2112 static int write_note_info(struct elf_note_info *info, ecc8c772 Al Viro 2013-10-05 2113 struct coredump_params *cprm) 3aba481f Roland McGrath 2008-01-30 2114 { 1e0d184d Alexey Dobriyan 2019-02-04 2115 struct elf_thread_status *ets; 3aba481f Roland McGrath 2008-01-30 2116 int i; 3aba481f Roland McGrath 2008-01-30 2117 3aba481f Roland McGrath 2008-01-30 2118 for (i = 0; i < info->numnote; i++) ecc8c772 Al Viro 2013-10-05 2119 if (!writenote(info->notes + i, cprm)) 3aba481f Roland McGrath 2008-01-30 2120 return 0; 3aba481f Roland McGrath 2008-01-30 2121 3aba481f Roland McGrath 2008-01-30 2122 /* write out the thread status notes section */ 1e0d184d Alexey Dobriyan 2019-02-04 2123 list_for_each_entry(ets, &info->thread_list, list) { 3aba481f Roland McGrath 2008-01-30 @2124 for (i = 0; i < tmp->num_notes; i++) ecc8c772 Al Viro 2013-10-05 2125 if (!writenote(&tmp->notes[i], cprm)) 3aba481f Roland McGrath 2008-01-30 2126 return 0; 3aba481f Roland McGrath 2008-01-30 2127 } 3aba481f Roland McGrath 2008-01-30 2128 3aba481f Roland McGrath 2008-01-30 2129 return 1; 3aba481f Roland McGrath 2008-01-30 2130 } 3aba481f Roland McGrath 2008-01-30 2131 :::::: The code at line 2124 was first introduced by commit :::::: 3aba481fc94d83ff630d4b7cd2f7447010c4c6df elf core dump: notes reorg :::::: TO: Roland McGrath <roland@redhat.com> :::::: CC: Ingo Molnar <mingo@elte.hu> --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 56297 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] elf: don't be afraid of overflow 2019-02-04 20:27 [PATCH 1/4] elf: don't be afraid of overflow Alexey Dobriyan 2019-02-04 20:28 ` [PATCH 2/4] elf: use list_for_each_entry() Alexey Dobriyan @ 2019-03-11 11:04 ` Pavel Machek 2019-03-11 17:10 ` Alexey Dobriyan 1 sibling, 1 reply; 9+ messages in thread From: Pavel Machek @ 2019-03-11 11:04 UTC (permalink / raw) To: Alexey Dobriyan; +Cc: akpm, linux-kernel [-- Attachment #1: Type: text/plain, Size: 829 bytes --] On Mon 2019-02-04 23:27:15, Alexey Dobriyan wrote: > Number of ELF program headers is 16-bit by spec, so total size > comfortably fits into "unsigned int". If it can't overflow, gcc should know too, and optimize checks out... right? > @@ -429,13 +430,9 @@ static struct elf_phdr *load_elf_phdrs(struct elfhdr *elf_ex, > goto out; > > /* Sanity check the number of program headers... */ > - if (elf_ex->e_phnum < 1 || > - elf_ex->e_phnum > 65536U / sizeof(struct elf_phdr)) > - goto out; > - > /* ...and their total size. */ This is just wrong. You removed check for zero, and I'm pretty sure sizeof() is not 1, so this one can trigger, too. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] elf: don't be afraid of overflow 2019-03-11 11:04 ` [PATCH 1/4] elf: don't be afraid of overflow Pavel Machek @ 2019-03-11 17:10 ` Alexey Dobriyan 2019-03-12 9:37 ` Pavel Machek 0 siblings, 1 reply; 9+ messages in thread From: Alexey Dobriyan @ 2019-03-11 17:10 UTC (permalink / raw) To: Pavel Machek; +Cc: akpm, linux-kernel On Mon, Mar 11, 2019 at 12:04:23PM +0100, Pavel Machek wrote: > On Mon 2019-02-04 23:27:15, Alexey Dobriyan wrote: > > Number of ELF program headers is 16-bit by spec, so total size > > comfortably fits into "unsigned int". > > If it can't overflow, gcc should know too, and optimize checks > out... right? Turns out it doesn't. > > @@ -429,13 +430,9 @@ static struct elf_phdr *load_elf_phdrs(struct elfhdr *elf_ex, > > goto out; > > > > /* Sanity check the number of program headers... */ > > - if (elf_ex->e_phnum < 1 || > > - elf_ex->e_phnum > 65536U / sizeof(struct elf_phdr)) > > - goto out; > > - > > /* ...and their total size. */ > > This is just wrong. You removed check for zero, and I'm pretty sure > sizeof() is not 1, so this one can trigger, too. No. ->e_phnum is 65535 max. size = sizeof(struct elf_phdr) * elf_ex->e_phnum; if (size == 0 || size > 65536 || size > ELF_MIN_ALIGN) goto out; ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] elf: don't be afraid of overflow 2019-03-11 17:10 ` Alexey Dobriyan @ 2019-03-12 9:37 ` Pavel Machek 0 siblings, 0 replies; 9+ messages in thread From: Pavel Machek @ 2019-03-12 9:37 UTC (permalink / raw) To: Alexey Dobriyan; +Cc: akpm, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1168 bytes --] On Mon 2019-03-11 20:10:06, Alexey Dobriyan wrote: > On Mon, Mar 11, 2019 at 12:04:23PM +0100, Pavel Machek wrote: > > On Mon 2019-02-04 23:27:15, Alexey Dobriyan wrote: > > > Number of ELF program headers is 16-bit by spec, so total size > > > comfortably fits into "unsigned int". > > > > If it can't overflow, gcc should know too, and optimize checks > > out... right? > > Turns out it doesn't. Tells me you are doing something wrong. > > > @@ -429,13 +430,9 @@ static struct elf_phdr *load_elf_phdrs(struct elfhdr *elf_ex, > > > goto out; > > > > > > /* Sanity check the number of program headers... */ > > > - if (elf_ex->e_phnum < 1 || > > > - elf_ex->e_phnum > 65536U / sizeof(struct elf_phdr)) > > > - goto out; > > > - > > > /* ...and their total size. */ > > > > This is just wrong. You removed check for zero, and I'm pretty sure > > sizeof() is not 1, so this one can trigger, too. > > No. ->e_phnum is 65535 max. That does not invalidate my argument. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-03-12 9:37 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-02-04 20:27 [PATCH 1/4] elf: don't be afraid of overflow Alexey Dobriyan 2019-02-04 20:28 ` [PATCH 2/4] elf: use list_for_each_entry() Alexey Dobriyan 2019-02-04 20:28 ` [PATCH 3/4] elf: spread const a little Alexey Dobriyan 2019-02-04 20:33 ` [PATCH 4/4] elf: save allocation per exec Alexey Dobriyan 2019-02-12 15:49 ` [PATCH 2/4] elf: use list_for_each_entry() kbuild test robot 2019-02-12 16:21 ` kbuild test robot 2019-03-11 11:04 ` [PATCH 1/4] elf: don't be afraid of overflow Pavel Machek 2019-03-11 17:10 ` Alexey Dobriyan 2019-03-12 9:37 ` Pavel Machek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).