Messages in this thread | | | Subject | Re: get_unmapped_area && in_ia32_syscall (Was: [PATCH] uprobes/x86: fix detection of 32-bit user mode) | From | Dmitry Safonov <> | Date | Wed, 28 Aug 2019 00:40:52 +0100 |
| |
-Cc my old @virtuozzo email. Previously it just ignored emails and now sends those ugly html replies. Sorry about that - I've updated .mailmap now.
On 8/27/19 6:03 PM, Dmitry Safonov wrote: > Hi Oleg, > > 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.. > > [..] >> 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(), but that would be not much better in > my POV. 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.. Except criu afair there are at least wine, dosemu. > I had it in my TODO to fix this :) > > Do I read the code properly and xol is always one page? > Could that page be reserved on the top of mmap_base/mmap_compat_base at > the binfmt loading time? (I would need than to add .mremap() for > restoring sake). Probably, not reserving it if personality doesn't allow > randomization or providing a way to disable it..
If no one has concerns over such approach, I'll cook a fix just after Plumbers week.
Thanks, Dmitry
| |