KOSAKI Motohiro wrote:
> I think you should read do_brk() itself. the spec is
>
> success case:
> return addr argument
>
> error case:
> return error code
>
> When does it return invalid address?
If addr argument is bogus.
load_elf_binary() in fs/binfmt_elf.c
758 if (unlikely (elf_brk > elf_bss)) {
759 unsigned long nbyte;
760
761 /* There was a PT_LOAD segment with p_memsz > p_filesz
762 before this one. Map anonymous pages, if needed,
763 and clear the area. */
764 retval = set_brk (elf_bss + load_bias,
765 elf_brk + load_bias);
766 if (retval) {
767 send_sig(SIGKILL, current, 0);
I think this function is using address information from untrusted source
(executable file). Thus, I think retval could be
ELF_PAGEALIGN(elf_bss + load_bias) if elf_bss + load_bias is bogus.
I'm OK with this if it is guaranteed that elf_bss + load_bias is always valid
and set_brk() never returns ELF_PAGEALIGN(elf_bss + load_bias).
768 goto out_free_dentry;
769 }
(...snipped...)
835 /*
836 * Check to see if the section's size will overflow the
837 * allowed task size. Note that p_filesz must always be
838 * <= p_memsz so it is only necessary to check p_memsz.
839 */
840 if (BAD_ADDR(k) || elf_ppnt->p_filesz > elf_ppnt->p_memsz ||
841 elf_ppnt->p_memsz > TASK_SIZE ||
842 TASK_SIZE - elf_ppnt->p_memsz < k) {
843 /* set_brk can never work. Avoid overflows. */
844 send_sig(SIGKILL, current, 0);
845 retval = -EINVAL;
Here setting -EINVAL. So why not to set a valid error code for set_brk() above?
846 goto out_free_dentry;
847 }
848
849 k = elf_ppnt->p_vaddr + elf_ppnt->p_filesz;
850
851 if (k > elf_bss)
852 elf_bss = k;
853 if ((elf_ppnt->p_flags & PF_X) && end_code < k)
854 end_code = k;
855 if (end_data < k)
856 end_data = k;
857 k = elf_ppnt->p_vaddr + elf_ppnt->p_memsz;
858 if (k > elf_brk)
859 elf_brk = k;
--
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
