1. 08 Feb, 2013 17 commits
    • Oleg Nesterov's avatar
      uprobes/x86: Change __skip_sstep() to actually skip the whole insn · cf31ec3f
      Oleg Nesterov authored
      __skip_sstep() doesn't update regs->ip. Currently this is correct
      but only "by accident" and it doesn't skip the whole insn. Change
      it to advance ->ip by the length of the detected 0x66*0x90 sequence.
      Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
      Acked-by: default avatarSrikar Dronamraju <srikar@linux.vnet.ibm.com>
      cf31ec3f
    • Oleg Nesterov's avatar
      uprobes: Teach handler_chain() to filter out the probed task · da1816b1
      Oleg Nesterov authored
      Currrently the are 2 problems with pre-filtering:
      
      1. It is not possible to add/remove a task (mm) after uprobe_register()
      
      2. A forked child inherits all breakpoints and uprobe_consumer can not
         control this.
      
      This patch does the first step to improve the filtering. handler_chain()
      removes the breakpoints installed by this uprobe from current->mm if all
      handlers return UPROBE_HANDLER_REMOVE.
      
      Note that handler_chain() relies on ->register_rwsem to avoid the race
      with uprobe_register/unregister which can add/del a consumer, or even
      remove and then insert the new uprobe at the same address.
      
      Perhaps we will add uprobe_apply_mm(uprobe, mm, is_register) and teach
      copy_mm() to do filter(UPROBE_FILTER_FORK), but I think this change makes
      sense anyway.
      
      Note: instead of checking the retcode from uc->handler, we could add
      uc->filter(UPROBE_FILTER_BPHIT). But I think this is not optimal to
      call 2 hooks in a row. This buys nothing, and if handler/filter do
      something nontrivial they will probably do the same work twice.
      Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
      Acked-by: default avatarSrikar Dronamraju <srikar@linux.vnet.ibm.com>
      da1816b1
    • Oleg Nesterov's avatar
      uprobes: Reintroduce uprobe_consumer->filter() · 8a7f2fa0
      Oleg Nesterov authored
      Finally add uprobe_consumer->filter() and change consumer_filter()
      to actually call this method.
      
      Note that ->filter() accepts mm_struct, not task_struct. Because:
      
      	1. We do not have for_each_mm_user(mm, task).
      
      	2. Even if we implement for_each_mm_user(), ->filter() can
      	   use it itself.
      
      	3. It is not clear who will actually need this interface to
      	   do the "nontrivial" filtering.
      
      Another argument is "enum uprobe_filter_ctx", consumer->filter() can
      use it to figure out why/where it was called. For example, perhaps
      we can add UPROBE_FILTER_PRE_REGISTER used by build_map_info() to
      quickly "nack" the unwanted mm's. In this case consumer should know
      that it is called under ->i_mmap_mutex.
      
      See the previous discussion at http://marc.info/?t=135214229700002
      Perhaps we should pass more arguments, vma/vaddr?
      
      Note: this patch obviously can't help to filter out the child created
      by fork(), this will be addressed later.
      Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
      Acked-by: default avatarSrikar Dronamraju <srikar@linux.vnet.ibm.com>
      8a7f2fa0
    • Oleg Nesterov's avatar
      uprobes: Rationalize the usage of filter_chain() · 806a98bd
      Oleg Nesterov authored
      filter_chain() was added into install_breakpoint/remove_breakpoint to
      simplify the initial changes but this is sub-optimal.
      
      This patch shifts the callsite to the callers, register_for_each_vma()
      and uprobe_mmap(). This way:
      
      - It will be easier to add the new arguments. This is the main reason,
        we can do more optimizations later.
      
      - register_for_each_vma(is_register => true) can be optimized, we only
        need to consult the new consumer. The previous consumers were already
        asked when they called uprobe_register().
      
      This patch also moves the MMF_HAS_UPROBES check from remove_breakpoint(),
      this allows to avoid the potentionally costly filter_chain(). Note that
      register_for_each_vma(is_register => false) doesn't really need to take
      ->consumer_rwsem, but I don't think it makes sense to optimize this and
      introduce filter_chain_lockless().
      Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
      Acked-by: default avatarSrikar Dronamraju <srikar@linux.vnet.ibm.com>
      806a98bd
    • Oleg Nesterov's avatar
      uprobes: Kill uprobes_mutex[], separate alloc_uprobe() and __uprobe_register() · 66d06dff
      Oleg Nesterov authored
      uprobe_register() and uprobe_unregister() are the only users of
      mutex_lock(uprobes_hash(inode)), and the only reason why we can't
      simply remove it is that we need to ensure that delete_uprobe() is
      not possible after alloc_uprobe() and before consumer_add().
      
      IOW, we need to ensure that when we take uprobe->register_rwsem
      this uprobe is still valid and we didn't race with _unregister()
      which called delete_uprobe() in between.
      
      With this patch uprobe_register() simply checks uprobe_is_active()
      and retries if it hits this very unlikely race. uprobes_mutex[] is
      no longer needed and can be removed.
      
      There is another reason for this change, prepare_uprobe() should be
      folded into alloc_uprobe() and we do not want to hold the extra locks
      around read_mapping_page/etc.
      Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
      Acked-by: default avatarAnton Arapov <anton@redhat.com>
      Acked-by: default avatarSrikar Dronamraju <srikar@linux.vnet.ibm.com>
      66d06dff
    • Oleg Nesterov's avatar
      uprobes: Introduce uprobe_is_active() · 06b7bcd8
      Oleg Nesterov authored
      The lifetime of uprobe->rb_node and uprobe->inode is not refcounted,
      delete_uprobe() is called when we detect that uprobe has no consumers,
      and it would be deadly wrong to do this twice.
      
      Change delete_uprobe() to WARN() if it was already called. We use
      RB_CLEAR_NODE() to mark uprobe "inactive", then RB_EMPTY_NODE() can
      be used to detect this case.
      
      RB_EMPTY_NODE() is not used directly, we add the trivial helper for
      the next change.
      Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
      Acked-by: default avatarAnton Arapov <anton@redhat.com>
      Acked-by: default avatarSrikar Dronamraju <srikar@linux.vnet.ibm.com>
      06b7bcd8
    • Oleg Nesterov's avatar
      uprobes: Kill uprobe_events, use RB_EMPTY_ROOT() instead · 441f1eb7
      Oleg Nesterov authored
      uprobe_events counts the number of uprobes in uprobes_tree but
      it is used as a boolean. We can use RB_EMPTY_ROOT() instead.
      
      Probably no_uprobe_events() added by this patch can have more
      callers, say, mmf_recalc_uprobes().
      Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
      Acked-by: default avatarAnton Arapov <anton@redhat.com>
      Acked-by: default avatarSrikar Dronamraju <srikar@linux.vnet.ibm.com>
      441f1eb7
    • Oleg Nesterov's avatar
      uprobes: Kill uprobe->copy_mutex · d4d3ccc6
      Oleg Nesterov authored
      Now that ->register_rwsem is safe under ->mmap_sem we can kill
      ->copy_mutex and abuse down_write(&uprobe->consumer_rwsem).
      
      This makes prepare_uprobe() even more ugly, but we should kill
      it anyway.
      Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
      Acked-by: default avatarSrikar Dronamraju <srikar@linux.vnet.ibm.com>
      d4d3ccc6
    • Oleg Nesterov's avatar
      uprobes: Kill UPROBE_RUN_HANDLER flag · bb929284
      Oleg Nesterov authored
      Simply remove UPROBE_RUN_HANDLER and the corresponding code.
      
      It can only help if uprobe has a single consumer, and in fact
      it is no longer needed after handler_chain() was changed to use
      ->register_rwsem, we simply can not race with uprobe_register().
      Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
      Acked-by: default avatarSrikar Dronamraju <srikar@linux.vnet.ibm.com>
      bb929284
    • Oleg Nesterov's avatar
      uprobes: Change filter_chain() to iterate ->consumers list · 1ff6fee5
      Oleg Nesterov authored
      Now that it safe to use ->consumer_rwsem under ->mmap_sem we can
      almost finish the implementation of filter_chain(). It still lacks
      the actual uc->filter(...) call but othewrwise it is ready, just
      it pretends that ->filter() always returns true.
      Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
      Acked-by: default avatarSrikar Dronamraju <srikar@linux.vnet.ibm.com>
      1ff6fee5
    • Oleg Nesterov's avatar
      uprobes: Introduce uprobe->register_rwsem · e591c8d7
      Oleg Nesterov authored
      Introduce uprobe->register_rwsem. It is taken for writing around
      __uprobe_register/unregister.
      
      Change handler_chain() to use this sem rather than consumer_rwsem.
      
      The main reason for this change is that we have the nasty problem
      with mmap_sem/consumer_rwsem dependency. filter_chain() needs to
      protect uprobe->consumers like handler_chain(), but they can not
      use the same lock. filter_chain() can be called under ->mmap_sem
      (currently this is always true), but we want to allow ->handler()
      to play with the probed task's memory, and this needs ->mmap_sem.
      
      Alternatively we could use srcu, but synchronize_srcu() is very
      slow and ->register_rwsem allows us to do more. In particular, we
      can teach handler_chain() to do remove_breakpoint() if this bp is
      "nacked" by all consumers, we know that we can't race with the
      new consumer which does uprobe_register().
      
      See also the next patches. uprobes_mutex[] is almost ready to die.
      Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
      Acked-by: default avatarSrikar Dronamraju <srikar@linux.vnet.ibm.com>
      e591c8d7
    • Oleg Nesterov's avatar
      uprobes: _register() should always do register_for_each_vma(true) · 9a98e03c
      Oleg Nesterov authored
      To support the filtering uprobe_register() should do
      register_for_each_vma(true) every time the new consumer comes,
      we need to install the previously nacked breakpoints.
      
      Note:
      	- uprobes_mutex[] should die, what it actually protects is
      	  alloc_uprobe().
      
      	- UPROBE_RUN_HANDLER should die too, obviously it can't work
      	  unless uprobe has a single consumer. The consumer should
      	  serialize with _register/_unregister itself. Or this flag
      	  should live in uprobe_consumer->state.
      
      	- Perhaps we can do some optimizations later. For example, if
      	  filter_chain() never returns false uprobe can record this
      	  fact and avoid the unnecessary register_for_each_vma().
      Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
      Acked-by: default avatarSrikar Dronamraju <srikar@linux.vnet.ibm.com>
      9a98e03c
    • Oleg Nesterov's avatar
      uprobes: _unregister() should always do register_for_each_vma(false) · 04aab9b2
      Oleg Nesterov authored
      uprobe_unregister() removes the breakpoints only if the last consumer
      goes away. To support the filtering it should do this every time, we
      want to remove the breakpoints which nobody else want to keep.
      
      Note: given that filter_chain() is not actually implemented, this patch
      itself doesn't change the behaviour yet, register_for_each_vma(false)
      is a heavy "nop" unless there are no more consumers.
      Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
      Acked-by: default avatarSrikar Dronamraju <srikar@linux.vnet.ibm.com>
      04aab9b2
    • Oleg Nesterov's avatar
      uprobes: Introduce filter_chain() · 63633cbf
      Oleg Nesterov authored
      Add the new helper filter_chain(). Currently it is only placeholder,
      the comment explains what is should do. We will change it later to
      consult every consumer to decide whether we need to install the swbp.
      Until then it works as if any consumer returns true, this matches the
      current behavior.
      
      Change install_breakpoint() to call filter_chain() instead of checking
      uprobe->consumers != NULL. We obviously need this, and this equally
      closes the race with _unregister().
      
      Change remove_breakpoint() to call this helper too. Currently this is
      pointless because remove_breakpoint() is only called when the last
      consumer goes away, but we will change this.
      Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
      Acked-by: default avatarSrikar Dronamraju <srikar@linux.vnet.ibm.com>
      63633cbf
    • Oleg Nesterov's avatar
      uprobes: Kill uprobe_consumer->filter() · fe20d71f
      Oleg Nesterov authored
      uprobe_consumer->filter() is pointless in its current form, kill it.
      
      We will add it back, but with the different signature/semantics. Perhaps
      we will even re-introduce the callsite in handler_chain(), but not to
      just skip uc->handler().
      Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
      Acked-by: default avatarSrikar Dronamraju <srikar@linux.vnet.ibm.com>
      fe20d71f
    • Oleg Nesterov's avatar
      uprobes: Kill the pointless inode/uc checks in register/unregister · f0744af7
      Oleg Nesterov authored
      register/unregister verifies that inode/uc != NULL. For what?
      This really looks like "hide the potential problem", the caller
      should pass the valid data.
      
      register() also checks uc->next == NULL, probably to prevent the
      double-register but the caller can do other stupid/wrong things.
      If we do this check, then we should document that uc->next should
      be cleared before register() and add BUG_ON().
      
      Also add the small comment about the i_size_read() check.
      Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
      Acked-by: default avatarSrikar Dronamraju <srikar@linux.vnet.ibm.com>
      f0744af7
    • Oleg Nesterov's avatar
      uprobes: Move __set_bit(UPROBE_SKIP_SSTEP) into alloc_uprobe() · bbc33d05
      Oleg Nesterov authored
      Cosmetic. __set_bit(UPROBE_SKIP_SSTEP) is the part of initialization,
      it is not clear why it is set in insert_uprobe().
      Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
      Acked-by: default avatarSrikar Dronamraju <srikar@linux.vnet.ibm.com>
      bbc33d05
  2. 06 Feb, 2013 23 commits