Messages in this thread | | | Date | Fri, 22 Nov 2013 04:29:34 +0100 | From | Andi Kleen <> | Subject | Re: [PATCH] Add a text_poke syscall |
| |
Thanks for the code review.
> This looks a bit confusing, afaics we can avoid the duplications: > > // also handles bp_target_mm == NULL case > if (user_mode_vm(regs) && current->mm != bp_target_mm) > return 0;
I had that originally in my code, but I was worried there were any threads with mm == NULL and user_mode_vm still true (maybe some BIOS code or somesuch)
So I ended up with the longer, but safer, variant.
> > + err = 0; > > + mutex_lock(&text_mutex); > > + bp_target_mm = current->mm; > > + bp_int3_addr = (u8 *)addr + 1; > > + __text_poke_bp(pages, > > + (unsigned long)addr & ~PAGE_MASK, > > + insn, len, handler); > > This doesn't look right if npages == 2 but get_user_pages_fast() returns 1.
True, i'll add an error out.
> > __text_poke() checks pages[1] != NULL, but in this case it assumes > that memcpy(vaddr, opcode, len) should fit into the 1st page. > > > Ingo, Andi, I do not think that it is good idea to implement this > via ptrace. If nothing else, you need to fork the tracer which can > do PTRACE_POKETEXT.
Yes I agree, ptrace is not the right way to do this.
-Andi
-- ak@linux.intel.com -- Speaking for myself only.
| |