1. 20 Nov, 2013 2 commits
    • Oleg Nesterov's avatar
      uprobes: Don't assume that arch_uprobe->insn/ixol is u8[MAX_UINSN_BYTES] · 803200e2
      Oleg Nesterov authored
      arch_uprobe should be opaque as much as possible to the generic
      code, but currently it assumes that insn/ixol must be u8[] of the
      known size. Remove this unnecessary dependency, we can use "&" and
      and sizeof() with the same effect.
      Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
      Acked-by: default avatarSrikar Dronamraju <srikar@linux.vnet.ibm.com>
      803200e2
    • Oleg Nesterov's avatar
      uprobes: Add uprobe_task->dup_xol_work/dup_xol_addr · 32473431
      Oleg Nesterov authored
      uprobe_task->vaddr is a bit strange. The generic code uses it only
      to pass the additional argument to arch_uprobe_pre_xol(), and since
      it is always equal to instruction_pointer() this looks even more
      strange.
      
      And both utask->vaddr and and utask->autask have the same scope,
      they only have the meaning when the task executes the probed insn
      out-of-line, so it is safe to reuse both in UTASK_RUNNING state.
      
      This all means that logically ->vaddr belongs to arch_uprobe_task
      and we should probably move it there, arch_uprobe_pre_xol() can
      record instruction_pointer() itself.
      
      OTOH, it is also used by uprobe_copy_process() and dup_xol_work()
      for another purpose, this doesn't look clean and doesn't allow to
      move this member into arch_uprobe_task.
      
      This patch adds the union with 2 anonymous structs into uprobe_task.
      
      The first struct is autask + vaddr, this way we "almost" move vaddr
      into autask.
      
      The second struct has 2 new members for uprobe_copy_process() paths:
      ->dup_xol_addr which can be used instead ->vaddr, and ->dup_xol_work
      which can be used to avoid kmalloc() and simplify the code.
      
      Note that this union will likely have another member(s), we need
      something like "private_data_for_handlers" so that the tracing
      handlers could use it to communicate with call_fetch() methods.
      Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
      Reviewed-by: default avatarMasami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
      Acked-by: default avatarSrikar Dronamraju <srikar@linux.vnet.ibm.com>
      32473431
  2. 11 Nov, 2013 1 commit
  3. 09 Nov, 2013 2 commits
    • Oleg Nesterov's avatar
      uprobes: Fix the memory out of bound overwrite in copy_insn() · 2ded0980
      Oleg Nesterov authored
      1. copy_insn() doesn't look very nice, all calculations are
         confusing and it is not immediately clear why do we read
         the 2nd page first.
      
      2. The usage of inode->i_size is wrong on 32-bit machines.
      
      3. "Instruction at end of binary" logic is simply wrong, it
         doesn't handle the case when uprobe->offset > inode->i_size.
      
         In this case "bytes" overflows, and __copy_insn() writes to
         the memory outside of uprobe->arch.insn.
      
         Yes, uprobe_register() checks i_size_read(), but this file
         can be truncated after that. All i_size checks are racy, we
         do this only to catch the obvious mistakes.
      
      Change copy_insn() to call __copy_insn() in a loop, simplify
      and fix the bytes/nbytes calculations.
      
      Note: we do not care if we read extra bytes after inode->i_size
      if we got the valid page. This is fine because the task gets the
      same page after page-fault, and arch_uprobe_analyze_insn() can't
      know how many bytes were actually read anyway.
      Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
      2ded0980
    • Oleg Nesterov's avatar
      uprobes: Fix the wrong usage of current->utask in uprobe_copy_process() · 70d7f987
      Oleg Nesterov authored
      Commit aa59c53f "uprobes: Change uprobe_copy_process() to dup
      xol_area" has a stupid typo, we need to setup t->utask->vaddr but
      the code wrongly uses current->utask.
      
      Even with this bug dup_xol_work() works "in practice", but only
      because get_unmapped_area(NULL, TASK_SIZE - PAGE_SIZE) likely
      returns the same address every time.
      Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
      70d7f987
  4. 07 Nov, 2013 9 commits
  5. 06 Nov, 2013 16 commits
  6. 05 Nov, 2013 9 commits
  7. 04 Nov, 2013 1 commit