Re: Is it legal to return positive value when do_execve() succeeds?

[Tetsuo Handa]

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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/