On Sun, Jun 21, 2020 at 05:42:55PM -0600, Theo de Raadt wrote: > Paul Irofti <p...@irofti.net> wrote: > > > > > > > > > ??n 22 iunie 2020 01:26:16 EEST, Christian Weisgerber <na...@mips.inka.de> > > a scris: > > >Christian Weisgerber: > > > > > >> I tweaked the patch locally to make _timekeep a visible global > > >> symbol in libc. > > >> > > >> Printing its value has revealed two issues: > > >> > > >> * The timekeep page is mapped to the same address for every process. > > >> It changes across reboots, but once running, it's always the same. > > >> kettenis suggested > > >> - vaddr_t va; > > >> + vaddr_t va = 0; > > >> in exec_timekeep_map(), but that doesn't make a difference. > > > > > >But that's the kernel mapping, and my observation concerns the > > >userland mapping. So based on this, I moved ps_timekeep up into > > >the fields of struct process that are zeroed on creation. > > >With that, _timekeep is always 0 for all processes. :-/ > > > > > > I don't understand what problem you are trying to solve. Is it that > > timekeep is the same? That's because we create only one page and the > > address gets copied on fork. The diff was not designed to have timekeep > > zero'd on every process so it doesn't account for it. > > > And I think you aren't listening. > > He is saying it is at the same VA in *every* userland process. Since most > processes do use this little system call execve, it is implausible for it > to be at the same place, just like it is implausible for the signal tramp > to be same place, or ld.so, or libc.
The code we are talking about is only called from inside this little system call execve by exec_timekeep_map() and fixup(). 683 /* setup new registers and do misc. setup. */ 684 if (pack.ep_emul->e_fixup != NULL) { 685 if ((*pack.ep_emul->e_fixup)(p, &pack) != 0) 686 goto free_pack_abort; 687 } ... 694 /* map the process's signal trampoline code */ 695 if (exec_sigcode_map(pr, pack.ep_emul)) 696 goto free_pack_abort; 697 /* map the process's timekeep page */ 698 if (exec_timekeep_map(pr)) 699 goto free_pack_abort; The timekeep map code is doing the same thing as the sigcode map. 880 int 881 exec_timekeep_map(struct process *pr) 882 { 883 size_t timekeep_sz = sizeof(struct timekeep); 884 885 /* 886 * Similar to the sigcode object, except that there is a single 887 * timekeep object, and not one per emulation. 888 */ 889 if (timekeep_object == NULL) { The timekeep_object is checked if allocated and if not it does a uvm_map(kernel_map). The timekeep_object is global so the if condition is only true once. Then a second call to uvm_map() sends it to the process space. Fixup is called before this, which I think is wrong now. 863 a->au_id = AUX_openbsd_timekeep; 864 a->au_v = p->p_p->ps_timekeep; 865 a++; It should be map-fixup rather than fixup-map, right? But even reversing the order leads to the same va address. 683 /* map the process's timekeep page */ 684 if (exec_timekeep_map(pr)) 685 goto free_pack_abort; 686 /* setup new registers and do misc. setup. */ 687 if (pack.ep_emul->e_fixup != NULL) { 688 if ((*pack.ep_emul->e_fixup)(p, &pack) != 0) 689 goto free_pack_abort; 690 } So I don't know why the address is not randomized, but I bet if I print pr->ps_sigcode somehow from userland, it will be the same. Paul