* [PATCH 0/1] Additional strict check on ELF file.
@ 2018-02-26 15:46 Ilya Smith
2018-02-26 15:46 ` [PATCH 1/1] Additional strict check on ELF file. Checks segments are followed in order of 'p_vaddr ' value ascending. It fixes erorr in total_mapping_size with computation total size. This error happens if segments in ELF file are not in order Ilya Smith
0 siblings, 1 reply; 4+ messages in thread
From: Ilya Smith @ 2018-02-26 15:46 UTC (permalink / raw)
To: viro, linux-fsdevel, linux-kernel; +Cc: Ilya Smith
Hello, this patch implements strict check on segments order in ELF file.
Current implementation doesn't check the order of ELF segments and compute
totalsize as difference between last one and the first one. If ELF file
has wrong order it allows to re-mmap existing memory region with one
from ELF file.
Ilya Smith (1):
Additional strict check on ELF file. Checks segments are followed in
order of 'p_vaddr ' value ascending. It fixes erorr in
total_mapping_size with computation total size. This error happens
if segments in ELF file are not in order.
fs/binfmt_elf.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
--
2.14.1
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH 1/1] Additional strict check on ELF file. Checks segments are followed in order of 'p_vaddr ' value ascending. It fixes erorr in total_mapping_size with computation total size. This error happens if segments in ELF file are not in order. 2018-02-26 15:46 [PATCH 0/1] Additional strict check on ELF file Ilya Smith @ 2018-02-26 15:46 ` Ilya Smith 2018-02-26 17:48 ` Randy Dunlap 0 siblings, 1 reply; 4+ messages in thread From: Ilya Smith @ 2018-02-26 15:46 UTC (permalink / raw) To: viro, linux-fsdevel, linux-kernel; +Cc: Ilya Smith Signed-off-by: Ilya Smith <blackzert@gmail.com> --- fs/binfmt_elf.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index bdb201230bae..970b42044240 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -524,6 +524,52 @@ static inline int arch_check_elf(struct elfhdr *ehdr, bool has_interp, #endif /* !CONFIG_ARCH_BINFMT_ELF_STATE */ +/** + * elf_check_phdr() - common check ELF program header. + * @phdr: The program header to check + * @phdr_num: Count of program headers in @phdr from elf header. + * + * Checks ELF binary meets specification. + * + * Return: Zero to proceed with ELF load, non-zero to faile the ELF load + * with that return code. + */ +static int elf_check_phdr(struct elf_phdr *phdr, unsigned long phdr_num) +{ + unsigned long i; + struct elf_phdr *eppnt = phdr; + Elf64_Addr curr_vaddr; + Elf64_Xword curr_memsz; + + /* Find first PT_LOAD entry */ + for (i = 0; i < phdr_num && eppnt->p_type != PT_LOAD; ++i, ++eppnt) + ; + + /* no any PT_LOAD */ + if (i == phdr_num) + return -EINVAL; + + curr_memsz = eppnt->p_memsz; + curr_vaddr = eppnt->p_vaddr; + + for (++i, ++eppnt; i < phdr_num; ++i, ++eppnt) { + if (eppnt->p_type != PT_LOAD) + continue; + + /* Check order of vaddr */ + if (eppnt->p_vaddr <= curr_vaddr) + return -EINVAL; + + /* Check overlapping */ + if (eppnt->p_vaddr < curr_vaddr + curr_memsz) + return -EINVAL; + + curr_memsz = eppnt->p_memsz; + curr_vaddr = eppnt->p_vaddr; + } + return 0; +} + /* This is much more generalized than the library routine read function, so we keep this separate. Technically the library read function is only provided so that we can read a.out libraries that have @@ -551,6 +597,8 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex, goto out; if (!interpreter->f_op->mmap) goto out; + if (elf_check_phdr(interp_elf_phdata, interp_elf_ex->e_phnum)) + goto out; total_size = total_mapping_size(interp_elf_phdata, interp_elf_ex->e_phnum); @@ -733,6 +781,9 @@ static int load_elf_binary(struct linux_binprm *bprm) if (!elf_phdata) goto out; + if (elf_check_phdr(&loc->elf_ex, loc->elf_ex.e_phnum)) + goto out; + elf_ppnt = elf_phdata; elf_bss = 0; elf_brk = 0; -- 2.14.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] Additional strict check on ELF file. Checks segments are followed in order of 'p_vaddr ' value ascending. It fixes erorr in total_mapping_size with computation total size. This error happens if segments in ELF file are not in order. 2018-02-26 15:46 ` [PATCH 1/1] Additional strict check on ELF file. Checks segments are followed in order of 'p_vaddr ' value ascending. It fixes erorr in total_mapping_size with computation total size. This error happens if segments in ELF file are not in order Ilya Smith @ 2018-02-26 17:48 ` Randy Dunlap 2018-02-27 12:45 ` Ilya Smith 0 siblings, 1 reply; 4+ messages in thread From: Randy Dunlap @ 2018-02-26 17:48 UTC (permalink / raw) To: Ilya Smith, viro, linux-fsdevel, linux-kernel On 02/26/2018 07:46 AM, Ilya Smith wrote: > Signed-off-by: Ilya Smith <blackzert@gmail.com> > --- > fs/binfmt_elf.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 51 insertions(+) > > diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c > index bdb201230bae..970b42044240 100644 > --- a/fs/binfmt_elf.c > +++ b/fs/binfmt_elf.c > @@ -524,6 +524,52 @@ static inline int arch_check_elf(struct elfhdr *ehdr, bool has_interp, > > #endif /* !CONFIG_ARCH_BINFMT_ELF_STATE */ > > +/** > + * elf_check_phdr() - common check ELF program header. > + * @phdr: The program header to check > + * @phdr_num: Count of program headers in @phdr from elf header. > + * > + * Checks ELF binary meets specification. > + * > + * Return: Zero to proceed with ELF load, non-zero to faile the ELF load fail > + * with that return code. > + */ > +static int elf_check_phdr(struct elf_phdr *phdr, unsigned long phdr_num) > +{ And it would be nicer/better to have all of the "intro" text in the body of this message instead of in another email. thanks, -- ~Randy ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] Additional strict check on ELF file. Checks segments are followed in order of 'p_vaddr ' value ascending. It fixes erorr in total_mapping_size with computation total size. This error happens if segments in ELF file are not in order. 2018-02-26 17:48 ` Randy Dunlap @ 2018-02-27 12:45 ` Ilya Smith 0 siblings, 0 replies; 4+ messages in thread From: Ilya Smith @ 2018-02-27 12:45 UTC (permalink / raw) To: Randy Dunlap; +Cc: viro, linux-fsdevel, linux-kernel Thanks for that, I will fix it in next patch. Any other feedback? Thanks, Ilya > 26 февр. 2018 г., в 20:48, Randy Dunlap <rdunlap@infradead.org> написал(а): > > On 02/26/2018 07:46 AM, Ilya Smith wrote: >> Signed-off-by: Ilya Smith <blackzert@gmail.com> >> --- >> fs/binfmt_elf.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 51 insertions(+) >> >> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c >> index bdb201230bae..970b42044240 100644 >> --- a/fs/binfmt_elf.c >> +++ b/fs/binfmt_elf.c >> @@ -524,6 +524,52 @@ static inline int arch_check_elf(struct elfhdr *ehdr, bool has_interp, >> >> #endif /* !CONFIG_ARCH_BINFMT_ELF_STATE */ >> >> +/** >> + * elf_check_phdr() - common check ELF program header. >> + * @phdr: The program header to check >> + * @phdr_num: Count of program headers in @phdr from elf header. >> + * >> + * Checks ELF binary meets specification. >> + * >> + * Return: Zero to proceed with ELF load, non-zero to faile the ELF load > > fail > >> + * with that return code. >> + */ >> +static int elf_check_phdr(struct elf_phdr *phdr, unsigned long phdr_num) >> +{ > > And it would be nicer/better to have all of the "intro" text in the body of this > message instead of in another email. > > thanks, > -- > ~Randy ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-02-27 12:45 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-02-26 15:46 [PATCH 0/1] Additional strict check on ELF file Ilya Smith 2018-02-26 15:46 ` [PATCH 1/1] Additional strict check on ELF file. Checks segments are followed in order of 'p_vaddr ' value ascending. It fixes erorr in total_mapping_size with computation total size. This error happens if segments in ELF file are not in order Ilya Smith 2018-02-26 17:48 ` Randy Dunlap 2018-02-27 12:45 ` Ilya Smith
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).