On 08/28, Dmitry Safonov wrote: > > > On 8/27/19 3:00 PM, Oleg Nesterov wrote: > > [..] > >> But to remind, there is another problem with in_ia32_syscall() && uprobes. > >> > >> get_unmapped_area() paths use in_ia32_syscall() and this is wrong in case > >> when the caller is xol_add_vma(), in this case TS_COMPAT won't be set.> > >> Usually the addr = TASK_SIZE - PAGE_SIZE passed to get_unmapped_area() > >> should > >> work, mm->get_unmapped_area() won't be even called. But if this addr is > >> already > >> occupied get_area() can return addr > TASK_SIZE. > > > > Technically, it's not bigger than TASK_SIZE that's supplied > > get_unmapped_area() as an argument..
Hmm. What do you mean? Just in case, TASK_SIZE checks TIF_ADDR32, not TS_COMPAT. > >> if (!area->vaddr) { > >> + if(!is_64bit_mm(mm)) > >> + current_thread_info()->status |= TS_COMPAT; > >> /* Try to map as high as possible, this is only a hint. */ > >> area->vaddr = get_unmapped_area(NULL, TASK_SIZE - PAGE_SIZE, > >> PAGE_SIZE, 0, 0); > >> + if(!is_64bit_mm(mm)) > >> + current_thread_info()->status &= ~TS_COMPAT;; > >> if (area->vaddr & ~PAGE_MASK) { > >> ret = area->vaddr; > >> goto fail; > > > > It could have been TASK_SIZE_OF(), tsk is always current, why do we need TASK_SIZE_OF() ? > > I see that arch_uprobe_analyze_insn() uses is_64bit_mm() which > > is correct the majority of time, but not for processes those jump > > switching CS.. Heh. it's actually even worse. Just suppose a 32-bit application simply mmaps a 64-bit executable which has a probe. But this is off-topic. > > Do I read the code properly and xol is always one page? Yes, > > Could that page be reserved on the top of mmap_base/mmap_compat_base at > > the binfmt loading time? How? I don't understand... > (I would need than to add .mremap() for > > restoring sake). for what? I don't think you can restore a probed process anyway... OK, right now this is off-topic too. Oleg.