Second try ...
Although it does not seem dangerous, I think it worth to fix this to avoid
potential future problems.
any thoughts on this ?
Thanks,
Hector.
> While working on a new ASLR for userspace we detected an error in the
> interpret loader.
>
> The size of the bss section for some interpreters is not correctly
> calculated resulting in unnecessary calls to vm_brk() with enormous size
> values.
>
> The bug appears when loading some interpreters with a small bss size. Once
> the last loadable segment has been loaded, the bss section is zeroed up to
> the page boundary and the elf_bss variable is updated to this new page
> boundary. Because of this update (alignment), the last_bss could be less
> than elf_bss and the subtraction "last_bss - elf_bss" value could overflow.
>
> Although it is quite easy to check this error, it has not been manifested
> because some peculiarities of the bug. Following is a brief description:
>
>
> $ size /lib32/ld-2.19.so
> text data bss dec hex filename
> 128456 2964 192 131612 2021c /lib32/ld-2.19.so
>
>
> An execution example with:
> - last_bss: 0xb7794938
> - elf_bss: 0xb7794878
>
>
> From fs/binfmt_elf.c:
> ---------------------------------------------------------------------------
> if (last_bss > elf_bss) {
> /*
> * Now fill out the bss section. First pad the last page up
> * to the page boundary, and then perform a mmap to make sure
> * that there are zero-mapped pages up to and including the
> * last bss page.
> */
> if (padzero(elf_bss)) {
> error = -EFAULT;
> goto out;
> }
>
> /* What we have mapped so far */
> elf_bss = ELF_PAGESTART(elf_bss + ELF_MIN_ALIGN - 1);
>
> <---------- elf_bss here is 0xb7795000
>
> /* Map the last of the bss segment */
> error = vm_brk(elf_bss, last_bss - elf_bss); <-- overflow!
> if (BAD_ADDR(error))
> goto out;
> }
>
> error = load_addr;
> out:
> return error;
> }
> ---------------------------------------------------------------------------
>
>
> The size value requested to the vm_brk() call (last_bss - elf_bss) is
> 0xfffffffffffff938 and internally this size is page aligned in the do_brk()
> function resulting in a 0 length request.
>
> static unsigned long do_brk(unsigned long addr, unsigned long len)
> {
> ...
> len = PAGE_ALIGN(len);
> if (!len)
> return addr;
>
>
> Since a segment of 0 bytes is perfectly valid, it returns the requested
> address to vm_brk() and because it is page aligned (done by the previous
> line to the vm_brk() call the "error" is not detected by
> "BAD_ADDR(error)" and the "load_elf_interp()" functions does not
> returns any error. Note that vm_brk() is not necessary at all.
>
> In brief, if the end of the bss is in the same page than the last segment
> loaded then the size of the last of bss segment is incorrectly calculated.
>
>
> This patch set up to the page boundary of the last_bss variable and only do
> the vm_brk() call when necessary.
>
>
> Signed-off-by: Hector Marco-Gisbert <hecmargi@xxxxxx>
> Acked-by: Ismael Ripoll Ripoll <iripoll@xxxxxx>
> ---
> fs/binfmt_elf.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 81381cc..acfbdc2 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -526,7 +526,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
> int load_addr_set = 0;
> unsigned long last_bss = 0, elf_bss = 0;
> unsigned long error = ~0UL;
> - unsigned long total_size;
> + unsigned long total_size, size;
> int i;
>
> /* First of all, some simple consistency checks */
> @@ -626,11 +626,15 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
>
> /* What we have mapped so far */
> elf_bss = ELF_PAGESTART(elf_bss + ELF_MIN_ALIGN - 1);
> + last_bss = ELF_PAGESTART(last_bss + ELF_MIN_ALIGN - 1);
>
> /* Map the last of the bss segment */
> - error = vm_brk(elf_bss, last_bss - elf_bss);
> - if (BAD_ADDR(error))
> - goto out;
> + size = last_bss - elf_bss;
> + if (size) {
> + error = vm_brk(elf_bss, size);
> + if (BAD_ADDR(error))
> + goto out;
> + }
> }
>
> error = load_addr;
>
--
Dr. Hector Marco-Gisbert @ http://hmarco.org/
Cyber Security Researcher @ http://cybersecurity.upv.es
Universitat Politècnica de València (Spain)
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
