On 10/16, Song Liu wrote: > > --- a/kernel/events/uprobes.c > +++ b/kernel/events/uprobes.c > @@ -474,14 +474,17 @@ int uprobe_write_opcode(struct arch_uprobe *auprobe, > struct mm_struct *mm, > struct vm_area_struct *vma; > int ret, is_register, ref_ctr_updated = 0; > bool orig_page_huge = false; > + unsigned int gup_flags = FOLL_FORCE; > > is_register = is_swbp_insn(&opcode); > uprobe = container_of(auprobe, struct uprobe, arch); > > retry: > + if (is_register) > + gup_flags |= FOLL_SPLIT_PMD; > /* Read the page with vaddr into memory */ > - ret = get_user_pages_remote(NULL, mm, vaddr, 1, > - FOLL_FORCE | FOLL_SPLIT_PMD, &old_page, &vma, NULL); > + ret = get_user_pages_remote(NULL, mm, vaddr, 1, gup_flags, > + &old_page, &vma, NULL); > if (ret <= 0) > return ret; > > @@ -489,6 +492,9 @@ int uprobe_write_opcode(struct arch_uprobe *auprobe, > struct mm_struct *mm, > if (ret <= 0) > goto put_old; > > + WARN(!is_register && PageCompound(old_page), > + "uprobe unregister should never work on compound page\n");
But this can happen with the change above. You can't know if *vaddr was previously changed by install_breakpoint() or not. If not, verify_opcode() should likely save us, but we can't rely on it. Say, someone can write "int3" into vm_file at uprobe->offset. And I am not sure it is safe to continue in this case, I'd suggest to return -EWHATEVER to avoid the possible crash. Oleg.
