Uprobes patches.

kernel/uprobes.c new file +8 -0

...
+static struct uprobe_process *find_uprocess(struct pid *tg_leader)
+{
+	struct uprobe_process *uproc;
+	struct task_struct *tsk = get_pid_task(tg_leader, PIDTYPE_PID);
+
+	if (!tsk)
+		return NULL;
+

kernel/uprobes.c new file +5 -0

+static void cleanup_uprocess(struct uprobe_process *uproc,
+					struct task_struct *start)
+{
+	struct task_struct *tsk = start;
+

kernel/uprobes.c new file +8 -0

+static struct uprobe_task *add_utask(struct task_struct *t,
+					struct uprobe_process *uproc)
+{
+	struct uprobe_task *utask;
+
+	if (!t)
+		return NULL;
+	utask = kzalloc(sizeof *utask, GFP_USER);

kernel/uprobes.c new file +13 -0

+static struct uprobe_process *create_uprocess(struct pid *tg_leader)
+{
...
+	/*
+	 * Create and populate one utask per thread in this process.  We
+	 * can't call add_utask() while holding RCU lock, so we:
+	 *	1. rcu_read_lock()
+	 *	2. Find the next thread, add_me, in this process that's not
+	 *	having a utask struct allocated.
+	 *	3. rcu_read_unlock()
+	 *	4. add_utask(add_me, uproc)
+	 *	Repeat 1-4 'til we have utasks for all threads.
+	 */
+	cur_t = get_pid_task(tg_leader, PIDTYPE_PID);

kernel/uprobes.c new file +24 -0

+int register_uprobe(struct uprobe *u)
+{
+	struct uprobe_process *uproc;
+	struct uprobe_probept *ppt;
+	struct pid *p;
+	int ret = 0;
+
+	if (!u || !u->handler)
+		return -EINVAL;
+
+	p = get_tg_leader(u->pid);
+	if (!p)
+		return -ESRCH;
+
+
+	/* Get the uprobe_process for this pid, or make a new one. */
+	mutex_lock(&uprobe_mutex);
+	uproc = find_uprocess(p);
+
+	if (!uproc) {
+		uproc = create_uprocess(p);
+		if (IS_ERR(uproc)) {
+			ret = (int) PTR_ERR(uproc);
+			mutex_unlock(&uprobe_mutex);

kernel/uprobes.c new file +14 -0

+void uprobe_handle_clone(unsigned long clone_flags,
+				struct task_struct *child)
+{
+	struct uprobe_process *uproc;
+	struct uprobe_task *ptask, *ctask;
+
+	ptask = current->utask;
+	if (!ptask)
+		return;
+
+	uproc = ptask->uproc;
+
+	if (clone_flags & CLONE_THREAD) {
+		mutex_lock(&uprobe_mutex);

kernel/uprobes.c new file +13 -0

+	} else {
+		struct uprobe_probept *ppt;
+		int ret;
+
+		/*
+		 * New process spawned by parent.  Remove the probepoints
+		 * in the child's text.
+		 *
+		 * We also hold the uproc->mutex for the parent - so no
+		 * new uprobes will be registered 'til we return.
+		 */
+		mutex_lock(&uproc->mutex);
+		ctask = child->utask;

Quoted 14 lines

...

kernel/uprobes.c new file +13 -0

+static struct uprobe_process *find_uprocess(struct pid *tg_leader)
+{
+	struct uprobe_process *uproc;
+	struct task_struct *tsk = get_pid_task(tg_leader, PIDTYPE_PID);
+
+	if (!tsk)
+		return NULL;
+
+	if (!tsk->utask || !tsk->utask->uproc) {
+		put_task_struct(tsk);
+		return NULL;
+	}
+

kernel/uprobes.c line 166

Quoted 5 lines

kernel/uprobes.c new file +5 -0

+	uproc = tsk->utask->uproc;
+	BUG_ON(uproc->tg_leader != tg_leader);
+	atomic_inc(&uproc->refcount);
+	put_task_struct(tsk);
+	return uproc;

Looks like, this doesn't need get/put task_struct, you could just use pid_task() under rcu_read_lock().

Quoted 12 lines

kernel/uprobes.c new file +11 -0

+static void cleanup_uprocess(struct uprobe_process *uproc,
+					struct task_struct *start)
+{
+	struct task_struct *tsk = start;
+
+	if (!start)
+		return;
+
+	rcu_read_lock();
+	do {
+		if (tsk->utask) {

kernel/uprobes.c line 212

Quoted 4 lines

kernel/uprobes.c new file +4 -0

+			kfree(tsk->utask);
+			tsk->utask = NULL;
+		}
+		tsk = next_thread(tsk);

This doesn't look right. We can't trust ->thread_group list even under rcu_read_lock(). The task can exit and __exit_signal() can remove it from ->thread_group list before we take rcu_read_lock().

Quoted 14 lines

kernel/uprobes.c new file +13 -0

+static struct uprobe_task *add_utask(struct task_struct *t,
+					struct uprobe_process *uproc)
+{
+	struct uprobe_task *utask;
+
+	if (!t)
+		return NULL;
+	utask = kzalloc(sizeof *utask, GFP_USER);
+	if (unlikely(utask == NULL))
+		return ERR_PTR(-ENOMEM);
+
+	utask->uproc = uproc;
+	utask->active_ppt = NULL;

kernel/uprobes.c line 275

Quoted 5 lines

kernel/uprobes.c new file +5 -0

+	t->utask = utask;
+	atomic_inc(&uproc->refcount);
+
+	return utask;
+}

This is called by create_uprocess(). Who will free t->utask if t has already passed tracehook_report_exit() ?

kernel/uprobes.c line 296

Quoted 13 lines

kernel/uprobes.c new file +12 -0

+static struct task_struct *find_next_thread(struct uprobe_process *uproc,
+						struct task_struct *start)
+{
+	struct task_struct *next_t = NULL;
+
+	rcu_read_lock();
+	if (start) {
+		struct task_struct *t = start;
+
+		do {
+			if (unlikely(t->flags & PF_EXITING))
+				goto dont_add;

not sure I understand this check. Somehow we should prevent the races with tracehook_report_exit/tracehook_report_exec, but PF_EXITING can't help ?

kernel/uprobes.c line 307

Quoted 4 lines

kernel/uprobes.c new file +3 -0

+dont_add:
+			t = next_thread(t);
+		} while (t != start);

again, this doesn't look right. Btw, I'd suggest to use while_each_thread().

Quoted 21 lines

kernel/uprobes.c new file +20 -0

+static struct uprobe_process *create_uprocess(struct pid *tg_leader)
+{
...
+	/*
+	 * Create and populate one utask per thread in this process.  We
+	 * can't call add_utask() while holding RCU lock, so we:
+	 *	1. rcu_read_lock()
+	 *	2. Find the next thread, add_me, in this process that's not
+	 *	having a utask struct allocated.
+	 *	3. rcu_read_unlock()
+	 *	4. add_utask(add_me, uproc)
+	 *	Repeat 1-4 'til we have utasks for all threads.
+	 */
+	cur_t = get_pid_task(tg_leader, PIDTYPE_PID);
+	do {
+		utask = add_utask(cur_t, uproc);
+		if (IS_ERR(utask)) {
+			err = PTR_ERR(utask);
+			goto fail;
+		}
+		add_me = find_next_thread(uproc, cur_t);

kernel/uprobes.c line 352

Quoted 3 lines

kernel/uprobes.c new file +3 -0

+		put_task_struct(cur_t);
+		cur_t = add_me;
+	} while (add_me != NULL);

can't we race with clone(CLONE_THREAD) and miss the new thread? Probably I missed something, but afaics we need some barriers to ensure that either tracehook_report_clone() sees current->utask != NULL or find_next_thread() sees the new thread in ->thread_group.

kernel/uprobes.c line 373

Quoted 8 lines

kernel/uprobes.c new file +7 -0

+static struct pid *get_tg_leader(pid_t p)
+{
+	struct pid *pid = NULL;
+
+	rcu_read_lock();
+	if (current->nsproxy)
+		pid = find_vpid(p);

Is it really possible to call register/unregister with nsproxy == NULL?

kernel/uprobes.c line 377

Quoted 4 lines

kernel/uprobes.c new file +4 -0

+	if (pid) {
+		struct task_struct *t = pid_task(pid, PIDTYPE_PID);
+
+		if (!t || unlikely(t->flags & PF_EXITING))

Why do we check PF_EXITING?

Quoted 25 lines

kernel/uprobes.c new file +24 -0

+int register_uprobe(struct uprobe *u)
+{
+	struct uprobe_process *uproc;
+	struct uprobe_probept *ppt;
+	struct pid *p;
+	int ret = 0;
+
+	if (!u || !u->handler)
+		return -EINVAL;
+
+	p = get_tg_leader(u->pid);
+	if (!p)
+		return -ESRCH;
+
+
+	/* Get the uprobe_process for this pid, or make a new one. */
+	mutex_lock(&uprobe_mutex);
+	uproc = find_uprocess(p);
+
+	if (!uproc) {
+		uproc = create_uprocess(p);
+		if (IS_ERR(uproc)) {
+			ret = (int) PTR_ERR(uproc);
+			mutex_unlock(&uprobe_mutex);

kernel/uprobes.c line 420

Quoted 10 lines

kernel/uprobes.c new file +10 -0

+			goto fail_tsk;
+		}
+	}
+	mutex_unlock(&uprobe_mutex);
+	mutex_lock(&uproc->mutex);
+
+	if (uproc->n_ppts >= MAX_USER_BKPT_XOL_SLOTS)
+		goto fail_uproc;
+
+	ret = xol_validate_vaddr(p, u->vaddr, uproc->xol_area);

OK, uproc and p can't go away. But why it is safe to use pid_task(p) ?

I am looking at 6th patch http://marc.info/?l=linux-kernel&m=127005086102256
and xol_validate_vaddr() calls pid_task() without rcu and doesn't check the result is not NULL.

We already dropped uprobe_mutex, can't this task exit?

Quoted 15 lines

kernel/uprobes.c new file +14 -0

+void uprobe_handle_clone(unsigned long clone_flags,
+				struct task_struct *child)
+{
+	struct uprobe_process *uproc;
+	struct uprobe_task *ptask, *ctask;
+
+	ptask = current->utask;
+	if (!ptask)
+		return;
+
+	uproc = ptask->uproc;
+
+	if (clone_flags & CLONE_THREAD) {
+		mutex_lock(&uprobe_mutex);

kernel/uprobes.c line 623

Quoted 10 lines

kernel/uprobes.c new file +10 -0

+		/* New thread in the same process. */
+		ctask = child->utask;
+		if (unlikely(ctask)) {
+			/*
+			 * create_uprocess() ran just as this clone
+			 * happened, and has already accounted for the
+			 * new child.
+			 */
+		} else
+			ctask = add_utask(child, uproc);

This looks a bit strange. Why do we need "ctask" at all? It is not used, you could just do

	if (likely(!child->utask))
		add_utask(child, uproc);

The same for "else" branch.

Quoted 13 lines

kernel/uprobes.c new file +13 -0

+	} else {
+		struct uprobe_probept *ppt;
+		int ret;
+
+		/*
+		 * New process spawned by parent.  Remove the probepoints
+		 * in the child's text.
+		 *
+		 * We also hold the uproc->mutex for the parent - so no
+		 * new uprobes will be registered 'til we return.
+		 */
+		mutex_lock(&uproc->mutex);
+		ctask = child->utask;

kernel/uprobes.c line 647

Quoted 10 lines

kernel/uprobes.c new file +10 -0

+		if (unlikely(ctask)) {
+			/*
+			 * create_uprocess() ran just as this fork
+			 * happened, and has already created a new utask.
+			 */
+			mutex_unlock(&uproc->mutex);
+			return;
+		}
+		list_for_each_entry(ppt, &uproc->uprobe_list, ut_node) {
+			ret = user_bkpt_remove_bkpt(child, &ppt->user_bkpt);

OK, iiuc this should restore the original instruction, right?

But what about clone(CLONE_VM)? In this case this child shares ->mm with parent.

Quoted 12 lines

Quoted 12 lines

kernel/uprobes.c new file +11 -0

+static void cleanup_uprocess(struct uprobe_process *uproc,
+					struct task_struct *start)
+{
+	struct task_struct *tsk = start;
+
+	if (!start)
+		return;
+
+	rcu_read_lock();
+	do {
+		if (tsk->utask) {

Quoted 8 lines

kernel/uprobes.c line 212

Quoted 4 lines

kernel/uprobes.c new file +4 -0

+			kfree(tsk->utask);
+			tsk->utask = NULL;
+		}
+		tsk = next_thread(tsk);

This doesn't look right. We can't trust ->thread_group list even under rcu_read_lock(). The task can exit and __exit_signal() can remove it from ->thread_group list before we take rcu_read_lock().

Oh Okay, I get that the thread could be exiting from the time we allocated the utask to the time we are cleaning up here and hence we could be leaking utask.

Would it be okay if we explicitly (instead of the using tracehook_report_exit) call uprobe_free_utask() just after we set PF_EXITING. We could take the task_lock to synchronize with the add_utask() and do_exit().

Quoted 13 lines

Quoted 13 lines

kernel/uprobes.c new file +13 -0

+static struct uprobe_task *add_utask(struct task_struct *t,
+					struct uprobe_process *uproc)
+{
+	struct uprobe_task *utask;
+
+	if (!t)
+		return NULL;
+	utask = kzalloc(sizeof *utask, GFP_USER);
+	if (unlikely(utask == NULL))
+		return ERR_PTR(-ENOMEM);
+
+	utask->uproc = uproc;
+	utask->active_ppt = NULL;

Quoted 8 lines

kernel/uprobes.c line 275

Quoted 5 lines

kernel/uprobes.c new file +5 -0

+	t->utask = utask;
+	atomic_inc(&uproc->refcount);
+
+	return utask;
+}

This is called by create_uprocess(). Who will free t->utask if t has already passed tracehook_report_exit() ?

Okay.
Would it work if we

[... snip ...]

Quoted 17 lines

Quoted 17 lines

kernel/uprobes.c new file +17 -0

+	 * Create and populate one utask per thread in this process.  We
+	 * can't call add_utask() while holding RCU lock, so we:
+	 *	1. rcu_read_lock()
+	 *	2. Find the next thread, add_me, in this process that's not
+	 *	having a utask struct allocated.
+	 *	3. rcu_read_unlock()
+	 *	4. add_utask(add_me, uproc)
+	 *	Repeat 1-4 'til we have utasks for all threads.
+	 */
+	cur_t = get_pid_task(tg_leader, PIDTYPE_PID);
+	do {
+		utask = add_utask(cur_t, uproc);
+		if (IS_ERR(utask)) {
+			err = PTR_ERR(utask);
+			goto fail;
+		}
+		add_me = find_next_thread(uproc, cur_t);

Quoted 8 lines

kernel/uprobes.c line 352

Quoted 3 lines

kernel/uprobes.c new file +3 -0

+		put_task_struct(cur_t);
+		cur_t = add_me;
+	} while (add_me != NULL);

can't we race with clone(CLONE_THREAD) and miss the new thread? Probably I missed something, but afaics we need some barriers to ensure that either tracehook_report_clone() sees current->utask != NULL or find_next_thread() sees the new thread in ->thread_group.

The tracehook_report_clone is called after the element gets added to the thread_group list in copy_process().
Looking at three cases where current thread could be cloning a new thread.

a) current thread has a utask and tracehook_report_clone is not yet called.
utask for the new thread will be created by either
tracehook_report_clone or the find_next_thread whichever comes first.

Quoted 9 lines

kernel/uprobes.c line 373

Quoted 7 lines

kernel/uprobes.c new file +7 -0

+static struct pid *get_tg_leader(pid_t p)
+{
+	struct pid *pid = NULL;
+
+	rcu_read_lock();
+	if (current->nsproxy)
+		pid = find_vpid(p);

Is it really possible to call register/unregister with nsproxy == NULL?

Do you see a need for checking if the process is exiting before we place the probes?

Quoted 26 lines

Quoted 13 lines

kernel/uprobes.c new file +13 -0

+	} else {
+		struct uprobe_probept *ppt;
+		int ret;
+
+		/*
+		 * New process spawned by parent.  Remove the probepoints
+		 * in the child's text.
+		 *
+		 * We also hold the uproc->mutex for the parent - so no
+		 * new uprobes will be registered 'til we return.
+		 */
+		mutex_lock(&uproc->mutex);
+		ctask = child->utask;

kernel/uprobes.c line 647

Quoted 10 lines

kernel/uprobes.c new file +10 -0

+		if (unlikely(ctask)) {
+			/*
+			 * create_uprocess() ran just as this fork
+			 * happened, and has already created a new utask.
+			 */
+			mutex_unlock(&uproc->mutex);
+			return;
+		}
+		list_for_each_entry(ppt, &uproc->uprobe_list, ut_node) {
+			ret = user_bkpt_remove_bkpt(child, &ppt->user_bkpt);

OK, iiuc this should restore the original instruction, right?

Quoted 2 lines

But what about clone(CLONE_VM)? In this case this child shares ->mm with parent.

Okay, So I will remove the breakpoints only for ! CLONE(CLONE_VM) and !CLONE(CLONE_THREAD)
For CLONE(CLONE_VM) I will create a new uproc and utask structures. Since mm is shared; I guess the XOL vma gets shared between the processes.

Quoted 20 lines

Quoted 11 lines

Quoted 11 lines

kernel/uprobes.c new file +11 -0

+static void cleanup_uprocess(struct uprobe_process *uproc,
+					struct task_struct *start)
+{
+	struct task_struct *tsk = start;
+
+	if (!start)
+		return;
+
+	rcu_read_lock();
+	do {
+		if (tsk->utask) {

Quoted 8 lines

kernel/uprobes.c line 212

Quoted 4 lines

kernel/uprobes.c new file +4 -0

+			kfree(tsk->utask);
+			tsk->utask = NULL;
+		}
+		tsk = next_thread(tsk);

This doesn't look right. We can't trust ->thread_group list even under rcu_read_lock(). The task can exit and __exit_signal() can remove it from ->thread_group list before we take rcu_read_lock().

Quoted 8 lines

Oh Okay, I get that the thread could be exiting from the time we allocated the utask to the time we are cleaning up here and hence we could be leaking utask.

Would it be okay if we explicitly (instead of the using tracehook_report_exit) call uprobe_free_utask() just after we set PF_EXITING. We could take the task_lock to synchronize with the add_utask() and do_exit().

Not sure I understand....

I meant, it is not safe to use next_thread(tsk) if tsk was already removed from list by __unhash_process before we take rcu_read_lock().

Quoted 6 lines

Quoted 5 lines

can't we race with clone(CLONE_THREAD) and miss the new thread? Probably I missed something, but afaics we need some barriers to ensure that either tracehook_report_clone() sees current->utask != NULL or find_next_thread() sees the new thread in ->thread_group.

Quoted 8 lines

The tracehook_report_clone is called after the element gets added to the thread_group list in copy_process().
Looking at three cases where current thread could be cloning a new thread.

a) current thread has a utask and tracehook_report_clone is not yet called.
utask for the new thread will be created by either
tracehook_report_clone or the find_next_thread whichever comes first.

Yes, but my point was, we probably need mb's on both sides. Of course, this is only theoretical problem, but tracehook_report_clone() can read current->utask == NULL before the result of copy_process()->list_add_tail() is visible to another CPU which does create_uprocess().

Quoted 10 lines

Quoted 10 lines

kernel/uprobes.c line 373

Quoted 8 lines

kernel/uprobes.c new file +7 -0

+static struct pid *get_tg_leader(pid_t p)
+{
+	struct pid *pid = NULL;
+
+	rcu_read_lock();
+	if (current->nsproxy)
+		pid = find_vpid(p);

Is it really possible to call register/unregister with nsproxy == NULL?

You didn't answer ;)

Quoted 3 lines

Do you see a need for checking if the process is exiting before we place the probes?

Oh, I don't know. You are going to change this code anyway, I can't see in advance.

I tried to read the next 8/11 patch, and I have a couple more random questions.

- uprobe_process->tg_leader is not really used ?

- looks like, 7/11 can't be compiled without the next 8/11 ? say, the next patch defines arch_uprobe_disable_sstep() but it is used by 7/11

- I don't understand why do we need uprobe_{en,dis}able_interrupts helpers. pre_ssout() could just do local_irq_enable(). This path leads to get_signal_to_deliver() which enables irqs anyway, it is always safe to do this earlier and I don't think we need to disable irqs again later. In any case, I don't understand why these helpers use native_irq_xxx().

- pre_ssout() does .xol_vaddr = xol_get_insn_slot(). This looks a bit confusing, xol_get_insn_slot() should set .xol_vaddr correctly under lock.

	- pre_ssout() does user_bkpt_set_ip() after user_bkpt_pre_sstep().
	  Why? Shouldn't user_bkpt_pre_sstep() always set regs->ip ?
	  Otherwise uprobe_bkpt_notifier()->user_bkpt_pre_sstep() is not
	  right.

- I don't really understand why ->handler_in_interrupt is really useful, but never mind.

- However, handler_in_interrupt && !uses_xol_strategy() doesn't look right. uprobe_bkpt_notifier() is called with irqs disabled, right? but set_orig_insn() is might_sleep().

Quoted 9 lines

Quoted 4 lines

OK, iiuc this should restore the original instruction, right?

But what about clone(CLONE_VM)? In this case this child shares ->mm with parent.

Okay, So I will remove the breakpoints only for ! CLONE(CLONE_VM) and !CLONE(CLONE_THREAD)
For CLONE(CLONE_VM) I will create a new uproc and utask structures. Since mm is shared; I guess the XOL vma gets shared between the processes.

Yes, I think CLONE_VM without CLONE_THREAD needs utask too, but do we need the new uproc? OK, please forget about this for the moment.

Suppose that register_uprobe() succeeds and does set_bkpt(). What if another process (not sub-thread) with the same ->mm hits this bp? uprobe_bkpt_notifier() will see ->utask == NULL and return 0. Then do_int3() sends SIGTRAP and kills this task. OK, probably CLONE_VM alone is exotic, but CLONE_VFORK | VM is not.

I think uprobe_process should be per ->mm, not per-process.

I wonder if there any possibility to avoid task_struct->utask, or at least, if we can allocate it in uprobe_bkpt_notifier() on demand. Not sure.

Quoted 7 lines

Quoted 5 lines

Quoted 5 lines

kernel/uprobes.c line 212

Quoted 4 lines

kernel/uprobes.c new file +3 -0

+static void cleanup_uprocess(struct uprobe_process *uproc,
+					struct task_struct *start)
+		tsk = next_thread(tsk);

I meant, it is not safe to use next_thread(tsk) if tsk was already removed from list by __unhash_process before we take rcu_read_lock().

Okay, cleanup_process() gets called only and only if add_utask() fails to allocated utask struct.

Based on your inputs I will synchronize
exit_signals() and uprobe_free_utask(). However it still can happen that uprobe calls cleanup_uprocess() with reference to task struct which has just called __unhash_process(). Is there a way out of this?

Quoted 10 lines

Quoted 9 lines

Quoted 9 lines

kernel/uprobes.c line 373

Quoted 7 lines

kernel/uprobes.c new file +7 -0

+static struct pid *get_tg_leader(pid_t p)
+{
+	struct pid *pid = NULL;
+
+	rcu_read_lock();
+	if (current->nsproxy)
+		pid = find_vpid(p);

Is it really possible to call register/unregister with nsproxy == NULL?

Quoted 1 line

You didn't answer ;)

Can you please let me know when nsproxy is set to NULL?

If we are sure
that register/unregister will be called with nsproxy set, then I am happy to remove this check.

Quoted 1 line

- uprobe_process->tg_leader is not really used ?

Currently we have a reference to pid struct from the time we created a uprobe_process to the time we free the uprobe process.

So are you
suggesting that we dont have a reference to the pid structure or is that we dont need to cache the pid struct and access it thro task_pid(current) in free_uprobes()?

Quoted 6 lines

- I don't understand why do we need uprobe_{en,dis}able_interrupts helpers. pre_ssout() could just do local_irq_enable(). This path leads to get_signal_to_deliver() which enables irqs anyway, it is always safe to do this earlier and I don't think we need to disable irqs again later. In any case, I don't understand why these helpers use native_irq_xxx().

On i686, (unlike x86_64), do_notify_resume() gets called with irqs disabled. I had tried local_irq_enable couple of times but that didnt help probably because CONFIG_PARAVIRT is set in my .config and hence raw_local_irq_enable resolves to

static inline void raw_local_irq_enable(void)

{
	PVOP_VCALLEE0(pv_irq_ops.irq_enable);
}

What we need is the "sti" instruction. It looks like local_irq_enable actually doesnt do "sti". So I had to go back to using native_irq_enable().

Quoted 3 lines

- pre_ssout() does .xol_vaddr = xol_get_insn_slot(). This looks a bit confusing, xol_get_insn_slot() should set .xol_vaddr correctly under lock.

Can you please elaborate.

Quoted 2 lines

- I don't really understand why ->handler_in_interrupt is really useful, but never mind.

There is a small overhead when running the handlers in task context.

overhead of task over interrupt = (1.016851 - .907400) = .109451 usec % additional overhead = (.109451/.907400) * 100 = 12.062%

Quoted 4 lines

- However, handler_in_interrupt && !uses_xol_strategy() doesn't look right. uprobe_bkpt_notifier() is called with irqs disabled, right? but set_orig_insn() is might_sleep().

Yes, Uprobes currently supports only xol strategy. I.e I have
dropped single stepping inline strategy for uprobes. Hence when user_bkpt_pre_sstep gets called from uprobe_bkpt_notifier; we are sure that it doesnt call set_orig_insn().

Quoted 6 lines

Suppose that register_uprobe() succeeds and does set_bkpt(). What if another process (not sub-thread) with the same ->mm hits this bp? uprobe_bkpt_notifier() will see ->utask == NULL and return 0. Then do_int3() sends SIGTRAP and kills this task. OK, probably CLONE_VM alone is exotic, but CLONE_VFORK | VM is not.
...
I think uprobe_process should be per ->mm, not per-process.

True, One possibility could be to move the uprobe_process structure to mm_struct. But now sure if VM folks would be okay with that idea.

Quoted 2 lines

I wonder if there any possibility to avoid task_struct->utask, or at least, if we can allocate it in uprobe_bkpt_notifier() on demand. Not sure.

Except for the pointer to uprobe_process, all other fields in utask are per task. This per task information is mostly used at probe hit. Hence having it in task_struct makes it easily accessible. Do you have other ideas from where we could refer utask.

I did think about allocating a utask on the first hit of a breakpoint. However there are couple of issues.

1. Uprobes needs access to uprobe_process to search the breakpoints installed for that process. Currently we hang it out of utask. However if uprobe_process is made a part of mm_struct, this would no
more be an issue.

2. Currently when a breakpoint is hit, uprobes increments the refcount for the corresponding probepoint, and sets active_ppt in the utask for the current thread. This happens in interrupt context. Allocating utask on first breakpoint hit for that thread; has to be handled in task context.

If the utask has to be allocated, then uprobes has to search for the probepoint again in task context.
I dont think it would be an issue to search for the probepoint a
second time in the task context.

Quoted 5 lines · Srikar Dronamraju

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

kernel/uprobes.c new file +3 -0

+static void cleanup_uprocess(struct uprobe_process *uproc,
+					struct task_struct *start)
+		tsk = next_thread(tsk);

Quoted 5 lines

Quoted 2 lines

I meant, it is not safe to use next_thread(tsk) if tsk was already removed from list by __unhash_process before we take rcu_read_lock().

Okay, cleanup_process() gets called only and only if add_utask() fails to allocated utask struct.

Yes, but afaics we have the same issues in find_next_thread() called by create_uprocess().

Quoted 5 lines

Based on your inputs I will synchronize
exit_signals() and uprobe_free_utask(). However it still can happen that uprobe calls cleanup_uprocess() with reference to task struct which has just called __unhash_process(). Is there a way out of this?

In this particular case, probably we can rely on uprobe_mutex. Currently cleanup_uprocess() is called with start == cur_t. Instead, we should use the last task on which add_utask() succeeded, it can't exit (assuming we fix other discussed races with exit) because uprobe_free_utask() takes this mutex too.

However, perhaps it is better to rework this all. Say, can't we move uprobe_free_utask() into __put_task_struct() ? Afaics, this can greatly simplify things. If we add mm_struct->uproc, then utask doesn't need the pointer to uprobe_process.

Quoted 11 lines

Quoted 11 lines

Quoted 10 lines

Quoted 10 lines

kernel/uprobes.c line 373

Quoted 8 lines

kernel/uprobes.c new file +7 -0

+static struct pid *get_tg_leader(pid_t p)
+{
+	struct pid *pid = NULL;
+
+	rcu_read_lock();
+	if (current->nsproxy)
+		pid = find_vpid(p);

Is it really possible to call register/unregister with nsproxy == NULL?

Quoted 3 lines

Quoted 1 line

You didn't answer ;)

Can you please let me know when nsproxy is set to NULL?

exit_notify()->exit_task_namespaces()

Quoted 3 lines

If we are sure
that register/unregister will be called with nsproxy set, then I am happy to remove this check.

I think the exiting task shouldn't call register/unregister.

Quoted 5 lines

Quoted 2 lines

- uprobe_process->tg_leader is not really used ?

Currently we have a reference to pid struct from the time we created a uprobe_process to the time we free the uprobe process.

Yes, but

Quoted 4 lines

So are you
suggesting that we dont have a reference to the pid structure or is that we dont need to cache the pid struct and access it thro task_pid(current) in free_uprobes()?

I must have missed something. But I do not see where do we use uprobe_process->tg_leader. We never read it, apart from BUG_ON(uproc->tg_leader != tg_leader). No?

Quoted 18 lines

Quoted 7 lines

- I don't understand why do we need uprobe_{en,dis}able_interrupts helpers. pre_ssout() could just do local_irq_enable(). This path leads to get_signal_to_deliver() which enables irqs anyway, it is always safe to do this earlier and I don't think we need to disable irqs again later. In any case, I don't understand why these helpers use native_irq_xxx().

On i686, (unlike x86_64), do_notify_resume() gets called with irqs disabled. I had tried local_irq_enable couple of times but that didnt help probably because CONFIG_PARAVIRT is set in my .config and hence raw_local_irq_enable resolves to

static inline void raw_local_irq_enable(void)

{
	PVOP_VCALLEE0(pv_irq_ops.irq_enable);
}

Quoted 3 lines

What we need is the "sti" instruction. It looks like local_irq_enable actually doesnt do "sti". So I had to go back to using native_irq_enable().

Hmm. No, I can't explain this, I know nothing about paravirt. But this doesn't look right to me. Probably this should be discussed with paravirt developers...

Quoted 6 lines

Quoted 4 lines

- pre_ssout() does .xol_vaddr = xol_get_insn_slot(). This looks a bit confusing, xol_get_insn_slot() should set .xol_vaddr correctly under lock.

Can you please elaborate.

pre_ssout() does

	if (!user_bkpt.xol_vaddr)
		user_bkpt.xol_vaddr = xol_get_insn_slot();

but it could just do

	if (!user_bkpt.xol_vaddr)
		xol_get_insn_slot();

because xol_get_insn_slot() populates user_bkpt.xol_vaddr.

Btw. Why do we have the !CONFIG_USER_BKPT_XOL code in include/linux/user_bkpt_xol.h? CONFIG_UPROBES depends on CONFIG_USER_BKPT_XOL.

Also the declarations don't look nice... Probably I missed something, but why the code uses "void *" instead of "user_bkpt_xol_area *" for xol_area everywhere?

OK, even if "void *" makes sense for uproc->uprobe_process, why xol_alloc_area/xol_get_insn_slot/etc do not use "user_bkpt_xol_area *" ?

Quoted 4 lines

Quoted 2 lines

- I don't really understand why ->handler_in_interrupt is really useful, but never mind.

There is a small overhead when running the handlers in task context.

Sure, but

Quoted 2 lines

overhead of task over interrupt = (1.016851 - .907400) = .109451 usec % additional overhead = (.109451/.907400) * 100 = 12.062%

this overhead looks very minor. To me, it is better to simplify the code, at least in the first version.

That said, this is up to you, I am not asking you to remove this optimization. Just imho.

Quoted 6 lines

Quoted 5 lines

- However, handler_in_interrupt && !uses_xol_strategy() doesn't look right. uprobe_bkpt_notifier() is called with irqs disabled, right? but set_orig_insn() is might_sleep().

Quoted 4 lines

Yes, Uprobes currently supports only xol strategy. I.e I have
dropped single stepping inline strategy for uprobes. Hence when user_bkpt_pre_sstep gets called from uprobe_bkpt_notifier; we are sure that it doesnt call set_orig_insn().

OK, thanks. Perhaps a small comment to explain this makes sense...

Quoted 8 lines

Quoted 7 lines

Suppose that register_uprobe() succeeds and does set_bkpt(). What if another process (not sub-thread) with the same ->mm hits this bp? uprobe_bkpt_notifier() will see ->utask == NULL and return 0. Then do_int3() sends SIGTRAP and kills this task. OK, probably CLONE_VM alone is exotic, but CLONE_VFORK | VM is not.
...
I think uprobe_process should be per ->mm, not per-process.

Quoted 2 lines

True, One possibility could be to move the uprobe_process structure to mm_struct. But now sure if VM folks would be okay with that idea.

Yes, I was thinking about mm->struct->uproc too.

And, assuming we have this pointer in mm_struct:

Quoted 7 lines

Quoted 2 lines

I wonder if there any possibility to avoid task_struct->utask, or at least, if we can allocate it in uprobe_bkpt_notifier() on demand. Not sure.

Except for the pointer to uprobe_process, all other fields in utask are per task. This per task information is mostly used at probe hit. Hence having it in task_struct makes it easily accessible. Do you have other ideas from where we could refer utask.

Well, we could add the list of uprobe_task's into uprobe_process, it represents the tasks "inside" the probe hit. But yes, this is not easy, lets forget this, at least for now.

Quoted 7 lines

I did think about allocating a utask on the first hit of a breakpoint. However there are couple of issues.

1. Uprobes needs access to uprobe_process to search the breakpoints installed for that process. Currently we hang it out of utask. However if uprobe_process is made a part of mm_struct, this would no
more be an issue.

Yes,

Quoted 5 lines

2. Currently when a breakpoint is hit, uprobes increments the refcount for the corresponding probepoint, and sets active_ppt in the utask for the current thread. This happens in interrupt context. Allocating utask on first breakpoint hit for that thread; has to be handled in task context.

we could use GFP_ATOMIC, but I agree, this is not nice.

Quoted 4 lines

If the utask has to be allocated, then uprobes has to search for the probepoint again in task context.
I dont think it would be an issue to search for the probepoint a
second time in the task context.

Agreed. Although we need the new TIF_ bit for tracehook_notify_resume(), it can't trust "if (current->utask...)" checks.

Alternatively, without the "on demand" allocations, register_uprobe() has to find all threads which use the same ->mm and initialize ->utask. This is not easy.

Quoted 3 lines · Oleg Nesterov

I must have missed something. But I do not see where do we use uprobe_process->tg_leader. We never read it, apart from BUG_ON(uproc->tg_leader != tg_leader). No?

static int free_uprocess(struct uprobe_process *uproc)

{
	....
	put_pid(uproc->tg_leader);
        uproc->tg_leader = NULL;

}

Quoted 5 lines

Also the declarations don't look nice... Probably I missed something, but why the code uses "void *" instead of "user_bkpt_xol_area *" for xol_area everywhere?

OK, even if "void *" makes sense for uproc->uprobe_process,

why

Quoted 2 lines

xol_alloc_area/xol_get_insn_slot/etc do not use "user_bkpt_xol_area *" ?

user_bkpt_xol_area isn't exposed. This provides flexibility in changing the algorithm for more efficient slot allocation. Currently we allocate slots from just one page. Later on we could end-up having to allocate from more than contiguous pages. There was some discussion about allocating slots from TLS. So there is more than one reason that user_bkpt_xol can change. We could expose the struct and not access the fields directly but that would be hard to enforce.

Quoted 7 lines

Quoted 4 lines

If the utask has to be allocated, then uprobes has to search for the probepoint again in task context.
I dont think it would be an issue to search for the probepoint a
second time in the task context.

Agreed. Although we need the new TIF_ bit for tracehook_notify_resume(), it can't trust "if (current->utask...)" checks.

But do we need a new TIF bit? Can we just reuse the TIF_NOTIFY_RESUME flag that we use now?

Quoted 13 lines · Srikar Dronamraju

Quoted 3 lines · Oleg Nesterov

I must have missed something. But I do not see where do we use uprobe_process->tg_leader. We never read it, apart from BUG_ON(uproc->tg_leader != tg_leader). No?

static int free_uprocess(struct uprobe_process *uproc)

{
	....
	put_pid(uproc->tg_leader);
        uproc->tg_leader = NULL;

Quoted 1 line

}

Yes, yes, I see it does get/put pid. But where do we actually use uproc->tg_leader? Why it is needed at all?

Quoted 6 lines

Quoted 6 lines

Also the declarations don't look nice... Probably I missed something, but why the code uses "void *" instead of "user_bkpt_xol_area *" for xol_area everywhere?

OK, even if "void *" makes sense for uproc->uprobe_process,

                                         ^^^^^^^^^^^^^^^^^^^^^

I meant uprobe_process->xol_area

Quoted 11 lines

why

Quoted 2 lines

xol_alloc_area/xol_get_insn_slot/etc do not use "user_bkpt_xol_area *" ?

user_bkpt_xol_area isn't exposed. This provides flexibility in changing the algorithm for more efficient slot allocation. Currently we allocate slots from just one page. Later on we could end-up having to allocate from more than contiguous pages. There was some discussion about allocating slots from TLS. So there is more than one reason that user_bkpt_xol can change. We could expose the struct and not access the fields directly but that would be hard to enforce.

Still can't understand... Yes, we shouldn't expose the details, but we can just add "struct user_bkpt_xol_area;" into include file.

OK, this is minor.

Quoted 11 lines

Quoted 8 lines

Quoted 5 lines

If the utask has to be allocated, then uprobes has to search for the probepoint again in task context.
I dont think it would be an issue to search for the probepoint a
second time in the task context.

Agreed. Although we need the new TIF_ bit for tracehook_notify_resume(), it can't trust "if (current->utask...)" checks.

But do we need a new TIF bit? Can we just reuse the TIF_NOTIFY_RESUME flag that we use now?

Probably not... But somehow tracehook_notify_resume/uprobe_notify_resume should know we hit the bp and we need to allocate utask. Yes, tracehook_notify_resume() can always call uprobe_notify_resume() unconditionally, and uprobe_notify_resume() can notice the "find_probept() && !current->utask" case, but probably it is better to make this more explicit. And of course, the new bit should be set along with TIF_NOTIFY_RESUME.

Or. Instead of TIF_ bit, we can use something like

	#define UTASK_PLEASE_ALLOCATE_ME ((struct uprobe_task *)1)

uprobe_bkpt_notifier() sets current->utask = UTASK_PLEASE_ALLOCATE_ME, then tracehook_notify_resume/uprobe_notify_resume check this case.

I dunno, please do what you think right.

OK, the last questions:

1. Can't multiple write_opcode()'s race with each other?

Say, pre_ssout() calls remove_bkpt() lockless. can't it race with register_uprobe() which may write to the same page?

And, without uses_xol_strategy() there are more racy callers of write_opcode()... Probably something else.

2. Can't write_opcode() conflict with ksm doing replace_page() ?

3. mprotect(). write_opcode() checks !VM_WRITE. This is correct, otherwise we can race with the user-space writing to the same page.

But suppose that the application does mprotect(PROT_WRITE) after register_uprobe() installs the bp, now unregister_uprobe/etc can't restore the original insn?

4. mremap(). What if the application does mremap() and moves the memory? After that vaddr of user_bkpt/uprobe no longer matches the virtual address of bp. This breaks uprobe_bkpt_notifier(), unregister_uprobe(), etc.

Even worse. Say, unregister_uprobe() calls remove_bkpt(). mremap()+mmap() can be called after ->read_opcode() verifies vaddr points to bkpt_insn, but before write_opcode() changes the page.

Quoted 4 lines · Oleg Nesterov

3. mprotect(). write_opcode() checks !VM_WRITE. This is correct, otherwise we can race with the user-space writing to the same page.

Quoted 4 lines

But suppose that the application does mprotect(PROT_WRITE) after register_uprobe() installs the bp, now unregister_uprobe/etc can't restore the original insn?

I still need to verify this. I shall get back to you on this. However are there applications that mprotect(PROT_WRITE) text pages?

I am copying Mel Gorman and Andrea Arcangeli so that they can provide their inputs on VM and KSM related issues.

Quoted 12 lines · Srikar Dronamraju

Quoted 8 lines · Oleg Nesterov

3. mprotect(). write_opcode() checks !VM_WRITE. This is correct, otherwise we can race with the user-space writing to the same page.

But suppose that the application does mprotect(PROT_WRITE) after register_uprobe() installs the bp, now unregister_uprobe/etc can't restore the original insn?

Quoted 2 lines

I still need to verify this. I shall get back to you on this. However are there applications that mprotect(PROT_WRITE) text pages?

Well, I think the kernel should assume that the user-space can do anything.

Hmm. And if this vma is VM_SHARED, then this bp could be actually written to vm_file after mprotect().

But I think this doesn't really matter. When I actually look at patches 3 and 4, I am starting to think this all is very wrong.

Quoted 2 lines

I am copying Mel Gorman and Andrea Arcangeli so that they can provide their inputs on VM and KSM related issues.

Yes. We need vm experts here, I am not. Still, I'd like to share my concerns. I also added Rik and Hugh.

So, 3/11 does

	@@ -2617,7 +2617,10 @@ int replace_page(struct vm_area_struct *vma, struct page *page,
		}
	 
		get_page(kpage);
	-	page_add_anon_rmap(kpage, vma, addr);
	+	if (PageAnon(kpage))
	+		page_add_anon_rmap(kpage, vma, addr);
	+	else
	+		page_add_file_rmap(kpage);
	 
		flush_cache_page(vma, addr, pte_pfn(*ptep));
		ptep_clear_flush(vma, addr, ptep);

I see no point in this patch, please see below.

The next 4/11 patch introduces write_opcode() which roughly does:

	int write_opcode(unsigned long vaddr, user_bkpt_opcode_t opcode)
	{
		get_user_pages(write => false, &old_page);

		new_page = alloc_page_vma(...);

		... insert the bp into the new_page ...

		new_page->mapping = old_page->mapping;
		new_page->index = old_page->index;

		replace_page(old_page, new_page);
	}

This doesn't look right at all to me.

IF PageAnon(old_page):

in this case replace_page() calls page_add_anon_rmap() which needs the locked page.

ELSE:

I don't think the new page should evere preserve the mapping, this looks just wrong. It should be always anonymous.

And in fact, I do not understand why write_opcode() needs replace_page(). It could just use get_user_pages(FOLL_WRITE | FOLL_FORCE), no? It should create the anonymous page correctly.

Either way, I think register_uprobe() should disallow the probes in VM_SHARED/VM_MAYWRITE vmas.

Quoted 5 lines · Oleg Nesterov

Quoted 3 lines · Srikar Dronamraju

I still need to verify this. I shall get back to you on this. However are there applications that mprotect(PROT_WRITE) text pages?

Quoted 5 lines

Well, I think the kernel should assume that the user-space can do anything.

Hmm. And if this vma is VM_SHARED, then this bp could be actually written to vm_file after mprotect().

When I look through the load_.*_binary and load_.*_library functions, they seem to map the text regions MAP_PRIVATE|MAP_DENY_WRITE.

Also if vma are marked VM_SHARED and bp are inserted through ptrace, i.e(access_process_vm/get_user_pages), then we would still be writing to vm_file after mprotect?

Again, I am not sure if executable pages should be marked VM_SHARED.

Quoted 28 lines

	@@ -2617,7 +2617,10 @@ int replace_page(struct vm_area_struct *vma, struct page *page,
		}

		get_page(kpage);
	-	page_add_anon_rmap(kpage, vma, addr);
	+	if (PageAnon(kpage))
	+		page_add_anon_rmap(kpage, vma, addr);
	+	else
	+		page_add_file_rmap(kpage);

		flush_cache_page(vma, addr, pte_pfn(*ptep));
		ptep_clear_flush(vma, addr, ptep);

I see no point in this patch, please see below.

The next 4/11 patch introduces write_opcode() which roughly does:

	int write_opcode(unsigned long vaddr, user_bkpt_opcode_t opcode)
	{
		get_user_pages(write => false, &old_page);

		new_page = alloc_page_vma(...);

		... insert the bp into the new_page ...

		new_page->mapping = old_page->mapping;
		new_page->index = old_page->index;

Quoted 6 lines

		replace_page(old_page, new_page);
	}

This doesn't look right at all to me.

IF PageAnon(old_page):

		   ^^^ newpage

Quoted 7 lines

in this case replace_page() calls page_add_anon_rmap() which needs the locked page.

ELSE:

I don't think the new page should evere preserve the mapping, this looks just wrong. It should be always anonymous.

I did verify that page_add_file_rmap gets called from replace_page when we insert or remove a probe.

I would leave it for vm experts to decide what the right thing to do.

Quoted 3 lines

And in fact, I do not understand why write_opcode() needs replace_page(). It could just use get_user_pages(FOLL_WRITE | FOLL_FORCE), no? It should create the anonymous page correctly.

We were earlier doing access_process_vm that would inturn call get_user_pages to COW the page. However that needed that the threads of the target process be stopped.

Background page replacement was suggested by Linus and Peter. In this method.
1. we get a copy of the page.
2. modify the page
3. flush the tlbs.

Again, I am not sure if executable pages should be marked VM_SHARED.

Hmm. And if this vma is VM_SHARED, then this bp could be actually written to vm_file after mprotect().

Either way, I think register_uprobe() should disallow the probes in VM_SHARED/VM_MAYWRITE vmas.

Quoted 3 lines · Oleg Nesterov

Either way, I think register_uprobe() should disallow the probes in VM_SHARED/VM_MAYWRITE vmas.

VM_SHARED, fully agreed, MAYWRITE not so sure, MAP_PRIVATE has MAYWRITE iirc and its perfectly fine to poke at those.

Quoted 3 lines · Oleg Nesterov

Either way, I think register_uprobe() should disallow the probes in VM_SHARED/VM_MAYWRITE vmas.

VM_SHARED, fully agreed, MAYWRITE not so sure, MAP_PRIVATE has MAYWRITE iirc and its perfectly fine to poke at those.

But I still think this doesn't actually matter, replace_page() shouldn't preserve the mapping, it should always install the anonymous page. I can be wrong, of course.

so arguably it would be good to just make the rule be that you cannot probe a shared mapping. Because whatever you do, it's always the wrong thing.

But isn't text usually shared? I don't see how you could set any break points or jump probes on text pages with that restriction.

so arguably it would be good to just make the rule be that you cannot probe a shared mapping. Because whatever you do, it's always the wrong thing.

Quoted 4 lines

But I still think this doesn't actually matter, replace_page() shouldn't preserve the mapping, it should always install the anonymous page. I can be wrong, of course.

Well, if I reasd the patches right, uprobes will use "copy_to_user()" for the self-probing case. So that would definitely just modify a shared mapping.

Of course, arguably, who really cares? As long as it's not a security issue (and it isn't - since the person could just have written to the thing directly instead), I guess it doesn't much matter. But it's a bit sad when a probing feature either

- changes a global mapping that may be executed by other non-related processes that the prober isn't even _aware_ of.

- changes semantics by creating a non-coherent private page

so arguably it would be good to just make the rule be that you cannot probe a shared mapping. Because whatever you do, it's always the wrong thing.

Quoted 9 lines · Oleg Nesterov

4. mremap(). What if the application does mremap() and moves the memory? After that vaddr of user_bkpt/uprobe no longer matches the virtual address of bp. This breaks uprobe_bkpt_notifier(), unregister_uprobe(), etc.

Even worse. Say, unregister_uprobe() calls remove_bkpt(). mremap()+mmap() can be called after ->read_opcode() verifies vaddr points to bkpt_insn, but before write_opcode() changes the page.

I dont think we handle this case now. I think even munmap of the region where there are probes inserted also can have the same problem.

Are there ways to handle this.
I think taking a write lock on mmap_sem instead of the read lock could handle this problem.

I am copying Mel Gorman and Andrea Arcangeli so that they can provide their inputs on VM and KSM related issues.

Quoted 15 lines · Srikar Dronamraju

Quoted 9 lines · Oleg Nesterov

4. mremap(). What if the application does mremap() and moves the memory? After that vaddr of user_bkpt/uprobe no longer matches the virtual address of bp. This breaks uprobe_bkpt_notifier(), unregister_uprobe(), etc.

Even worse. Say, unregister_uprobe() calls remove_bkpt(). mremap()+mmap() can be called after ->read_opcode() verifies vaddr points to bkpt_insn, but before write_opcode() changes the page.

I dont think we handle this case now. I think even munmap of the region where there are probes inserted also can have the same problem.

Quoted 6 lines

Are there ways to handle this.
I think taking a write lock on mmap_sem instead of the read lock could handle this problem.

I am copying Mel Gorman and Andrea Arcangeli so that they can provide their inputs on VM and KSM related issues.

KSM only does anonymous pages, and I thought uprobes was limited to MAP_PRIVATE|PROT_EXEC file maps.

We can't hold mmap_sem (for either read or write -- read would be sufficient to serialize against mmap/mremap/munmap) from atomic uprobe context, what we can do is validate that there is a INT3 on that particular address, a mremap/munmap/munmap+mmap will either end not having a pte entry for the address, or not have the INT3.

That said, you shouldn't be executing code on maps you're changing, much fun can happen if you try, so I don't think we should expend too much effort as long as the race will only result in the app crashing and not the kernel.

Quoted 5 lines

We can't hold mmap_sem (for either read or write -- read would be sufficient to serialize against mmap/mremap/munmap) from atomic uprobe context, what we can do is validate that there is a INT3 on that particular address, a mremap/munmap/munmap+mmap will either end not having a pte entry for the address, or not have the INT3.

Did you mean "We can hold mmap_sem?" Else I am not sure if we can traverse the vma. Infact alloc_page_vma() needs mmap_sem to be acquired. Please clarify?

Quoted 6 lines · Srikar Dronamraju

Quoted 5 lines

We can't hold mmap_sem (for either read or write -- read would be sufficient to serialize against mmap/mremap/munmap) from atomic uprobe context, what we can do is validate that there is a INT3 on that particular address, a mremap/munmap/munmap+mmap will either end not having a pte entry for the address, or not have the INT3.

Quoted 3 lines

Did you mean "We can hold mmap_sem?" Else I am not sure if we can traverse the vma. Infact alloc_page_vma() needs mmap_sem to be acquired. Please clarify?

OK, so maybe I misunderstood, this is from the INT3 trap handler, right?

Quoted 9 lines

4. mremap(). What if the application does mremap() and moves the memory? After that vaddr of user_bkpt/uprobe no longer matches the virtual address of bp. This breaks uprobe_bkpt_notifier(), unregister_uprobe(), etc.

Even worse. Say, unregister_uprobe() calls remove_bkpt(). mremap()+mmap() can be called after ->read_opcode() verifies vaddr
points to bkpt_insn, but before write_opcode() changes the page.

We can _not_ take a sleeping lock from trap context. Why would you want the vma anyway?

Quoted 4 lines

Quoted 2 lines

- I don't really understand why ->handler_in_interrupt is really useful, but never mind.

There is a small overhead when running the handlers in task context.

Sure, but

Quoted 2 lines

overhead of task over interrupt = (1.016851 - .907400) = .109451 usec % additional overhead = (.109451/.907400) * 100 = 12.062%

this overhead looks very minor. To me, it is better to simplify the code, at least in the first version.

That said, this is up to you, I am not asking you to remove this optimization. Just imho.

[...]
Right so what I've suggested several times it to simply call the same handler in both contexts. If it returns -EFAULT, set TIF_UPROBE or whatever and try again from task context.

Hi -

Quoted 4 lines · Peter Zijlstra

[...]
Right so what I've suggested several times it to simply call the same handler in both contexts. If it returns -EFAULT, set TIF_UPROBE or whatever and try again from task context.

That could work, but random partial execution & restart of the handler will make it tricky to write a single handler that reliably produces results. It would likely need a flag to indicate that it failed previously so as to throw away partial results.

kernel/trace/trace.h +5 -0

+
+#define SIZEOF_UPROBE_TRACE_ENTRY(n)			\
+	(offsetof(struct uprobe_trace_entry, args) +	\
+	(sizeof(unsigned long) * (n)))
+
 /*
  * trace_flag_type is an enumeration that holds different
  * states when a trace occurs. These are:

kernel/trace/trace_uprobe.c new file +720 -0

new file mode 100644
index 0000000..3e146ef
@@ -0,0 +1,926 @@
+/*
+ * Uprobes-based tracing events
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ * Copyright (C) IBM Corporation, 2010
+ * Author: 	Srikar Dronamraju
+ */
+
+#include <linux/module.h>
+#include <linux/uaccess.h>
+#include <linux/uprobes.h>
+#include <linux/seq_file.h>
+#include <linux/debugfs.h>
+#include <linux/types.h>
+#include <linux/string.h>
+#include <linux/ctype.h>
+#include <linux/ptrace.h>
+#include <linux/perf_event.h>
+
+#include "trace.h"
+#include "trace_output.h"
+
+#define MAX_TRACE_ARGS 128
+#define MAX_ARGSTR_LEN 63
+#define MAX_EVENT_NAME_LEN 64
+#define UPROBE_EVENT_SYSTEM "uprobes"
+
+/* Reserved field names */
+#define FIELD_STRING_IP "__probe_ip"
+#define FIELD_STRING_NARGS "__probe_nargs"
+#define FIELD_STRING_PID "__probe_pid"
+
+const char *ureserved_field_names[] = {
+	"common_type",
+	"common_flags",
+	"common_preempt_count",
+	"common_pid",
+	"common_tgid",
+	"common_lock_depth",
+	FIELD_STRING_IP,
+	FIELD_STRING_NARGS,
+	FIELD_STRING_PID,
+};
+
+struct fetch_func {
+	unsigned long (*func)(struct pt_regs *, void *);
+	void *data;
+};
+
+static unsigned long call_fetch(struct fetch_func *f,
+					  struct pt_regs *regs)
+{
+	return f->func(regs, f->data);
+}
+
+/* fetch handlers */
+static  unsigned long fetch_register(struct pt_regs *regs,
+					      void *offset)
+{
+	return regs_get_register(regs, (unsigned int)((unsigned long)offset));
+}
+
+/**
+ * Uprobe event core functions
+ */
+
+/* Flags for trace_probe */
+#define TP_FLAG_TRACE	1
+#define TP_FLAG_PROFILE	2
+#define UPROBE_ENABLED	4
+
+struct probe_arg {
+	struct fetch_func	fetch;
+	const char		*name;
+};
+
+struct trace_uprobe {
+	struct list_head	list;
+	struct uprobe		up;
+	unsigned long 		nhit;
+	unsigned int		flags;	/* For TP_FLAG_* */
+	struct ftrace_event_call	call;
+	struct trace_event		event;
+	unsigned int		nr_args;
+	struct probe_arg	args[];
+};
+
+#define SIZEOF_TRACE_UPROBE(n)			\
+	(offsetof(struct trace_uprobe, args) +	\
+	(sizeof(struct probe_arg) * (n)))
+
+static int probe_arg_string(char *buf, size_t n, struct fetch_func *ff)
+{
+	int ret = -EINVAL;
+
+	if (ff->func == fetch_register) {
+		const char *name;
+		name = regs_query_register_name((unsigned int)((long)ff->data));
+		ret = snprintf(buf, n, "%%%s", name);
+	}
+	if (ret >= n)
+		return -ENOSPC;
+	return ret;
+}
+
+static int register_uprobe_event(struct trace_uprobe *tp);
+static void unregister_uprobe_event(struct trace_uprobe *tp);
+
+static DEFINE_MUTEX(uprobe_lock);
+static LIST_HEAD(uprobe_list);
+
+static void uprobe_dispatcher(struct uprobe *up, struct pt_regs *regs);
+
+/* Check the name is good for event/group */
+static int check_event_name(const char *name)
+{
+	if (!isalpha(*name) && *name != '_')
+		return 0;
+	while (*++name != '\0') {
+		if (!isalpha(*name) && !isdigit(*name) && *name != '_')
+			return 0;
+	}
+	return 1;
+}
+
+/*
+ * Allocate new trace_uprobe and initialize it (including uprobes).
+ */
+static struct trace_uprobe *alloc_trace_uprobe(const char * group,
+					const char *event,
+					void *addr,
+					pid_t pid, int nargs)
+{
+	struct trace_uprobe *tp;
+	int ret = -ENOMEM;
+
+	if (!event || !check_event_name(event))
+		return ERR_PTR(-EINVAL);
+	
+	if (!group || !check_event_name(group))
+		return ERR_PTR(-EINVAL);
+
+	tp = kzalloc(SIZEOF_TRACE_UPROBE(nargs), GFP_KERNEL);
+	if (!tp)
+		return ERR_PTR(ret);
+
+	tp->up.vaddr = (unsigned long)addr;
+	tp->up.pid = pid;
+	tp->up.handler = uprobe_dispatcher;
+
+	tp->call.name = kstrdup(event, GFP_KERNEL);
+	if (!tp->call.name)
+		goto error;
+
+	tp->call.system = kstrdup(group, GFP_KERNEL);
+	if (!tp->call.system)
+		goto error;
+
+	INIT_LIST_HEAD(&tp->list);
+	return tp;
+error:
+	kfree(tp->call.name);
+	kfree(tp);
+	return ERR_PTR(ret);
+}
+
+static void free_probe_arg(struct probe_arg *arg)
+{
+	kfree(arg->name);
+}
+
+static void free_trace_uprobe(struct trace_uprobe *tp)
+{
+	int i;
+
+	for (i = 0; i < tp->nr_args; i++)
+		free_probe_arg(&tp->args[i]);
+
+	kfree(tp->call.system);
+	kfree(tp->call.name);
+	kfree(tp);
+}
+
+static struct trace_uprobe *find_probe_event(const char *event,
+					const char *group)
+{
+	struct trace_uprobe *tp;
+
+	list_for_each_entry(tp, &uprobe_list, list)
+		if (strcmp(tp->call.name, event) == 0 &&
+		    strcmp(tp->call.system, group) == 0)
+			return tp;
+	return NULL;
+}
+
+/* Unregister a trace_uprobe and probe_event: call with locking uprobe_lock */
+static void unregister_trace_uprobe(struct trace_uprobe *tp)
+{
+	if (tp->flags & UPROBE_ENABLED)
+		unregister_uprobe(&tp->up);
+	list_del(&tp->list);
+	unregister_uprobe_event(tp);
+}
+
+/* Register a trace_uprobe and probe_event */
+static int register_trace_uprobe(struct trace_uprobe *tp)
+{
+	struct trace_uprobe *old_tp;
+	int ret;
+
+	mutex_lock(&uprobe_lock);
+
+	/* register as an event */
+	old_tp = find_probe_event(tp->call.name, tp->call.system);
+	if (old_tp) {
+		/* delete old event */
+		unregister_trace_uprobe(old_tp);
+		free_trace_uprobe(old_tp);
+	}
+	ret = register_uprobe_event(tp);
+	if (ret) {
+		pr_warning("Faild to register probe event(%d)\n", ret);
+		goto end;
+	}
+
+	list_add_tail(&tp->list, &uprobe_list);
+end:
+	mutex_unlock(&uprobe_lock);
+	return ret;
+}
+
+#define PARAM_MAX_ARGS 16
+#define PARAM_MAX_STACK (THREAD_SIZE / sizeof(unsigned long))
+
+/* Recursive argument parser */
+static int __parse_probe_arg(char *arg, struct fetch_func *ff)
+{
+	int ret = 0;
+
+	switch (arg[0]) {
+	case '%':	/* named register */
+		ret = regs_query_register_offset(arg + 1);
+		if (ret >= 0) {
+			ff->func = fetch_register;
+			ff->data = (void *)(unsigned long)ret;
+			ret = 0;
+		}
+		break;
+	default:
+		/* TODO: support custom handler */
+		ret = -EINVAL;
+	}
+	return ret;
+}
+
+/* String length checking wrapper */
+static int parse_probe_arg(char *arg, struct fetch_func *ff)
+{
+	if (strlen(arg) > MAX_ARGSTR_LEN) {
+		pr_info("Argument is too long.: %s\n",  arg);
+		return -ENOSPC;
+	}
+	return __parse_probe_arg(arg, ff);
+}
+
+/* Return 1 if name is reserved or already used by another argument */
+static int conflict_field_name(const char *name,
+			       struct probe_arg *args, int narg)
+{
+	int i;
+	for (i = 0; i < ARRAY_SIZE(ureserved_field_names); i++)
+		if (strcmp(ureserved_field_names[i], name) == 0)
+			return 1;
+	for (i = 0; i < narg; i++)
+		if (strcmp(args[i].name, name) == 0)
+			return 1;
+	return 0;
+}
+
+static int create_trace_uprobe(int argc, char **argv)
+{
+	/*
+	 * Argument syntax:
+	 *  - Add uprobe: p[:[GRP/]EVENT] VADDR@PID [%REG]
+	 *
+	 *  - Remove uprobe: -:[GRP/]EVENT
+	 */
+	struct trace_uprobe *tp;
+	int i, ret = 0;
+	int is_delete = 0;
+	char *arg = NULL, *event = NULL, *group = NULL;
+	void *addr = NULL;
+	pid_t pid = 0;
+	char buf[MAX_EVENT_NAME_LEN];
+
+	/* argc must be >= 1 */
+	if (argv[0][0] == '-')
+		is_delete = 1;
+	else if (argv[0][0] != 'p') {
+		pr_info("Probe definition must be started with 'p', 'r' or"
+			" '-'.\n");
+		return -EINVAL;
+	}
+
+	if (argv[0][1] == ':') {
+		event = &argv[0][2];
+		if (strchr(event, '/')) {
+			group = event;
+			event = strchr(group, '/') + 1;
+			event[-1] = '\0';
+			if (strlen(group) == 0) {
+				pr_info("Group name is not specified\n");
+				return -EINVAL;
+			}
+		}
+		if (strlen(event) == 0) {
+			pr_info("Event name is not specified\n");
+			return -EINVAL;
+		}
+	}
+	if (!group)
+		group = UPROBE_EVENT_SYSTEM;
+
+	if (is_delete) {
+		if (!event) {
+			pr_info("Delete command needs an event name.\n");
+			return -EINVAL;
+		}
+		tp = find_probe_event(event, group);
+		if (!tp) {
+			pr_info("Event %s/%s doesn't exist.\n", group, event);
+			return -ENOENT;
+		}
+		/* delete an event */
+		unregister_trace_uprobe(tp);
+		free_trace_uprobe(tp);
+		return 0;
+	}
+
+	if (argc < 2) {
+		pr_info("Probe point is not specified.\n");
+		return -EINVAL;
+	}
+	if (isdigit(argv[1][0])) {
+		/* an address specified */
+		arg = strchr(argv[1], ':');
+		if (!arg)
+			goto fail_address_parse;
+
+		*arg++ = '\0';
+		ret = strict_strtoul(&argv[1][0], 0, (unsigned long *)&pid);
+		if (ret)
+			goto fail_address_parse;
+
+		ret = strict_strtoul(arg, 0, (unsigned long *)&addr);
+		if (ret)
+			goto fail_address_parse;
+	}
+	argc -= 2; argv += 2;
+
+	/* setup a probe */
+	if (!event) {
+		snprintf(buf, MAX_EVENT_NAME_LEN, "%c_%d_0x%p", 'p',
+							pid, addr);
+		event = buf;
+	}
+	tp = alloc_trace_uprobe(group, event, addr, pid, argc);
+	if (IS_ERR(tp)) {
+		pr_info("Failed to allocate trace_uprobe.(%d)\n",
+			(int)PTR_ERR(tp));
+		return PTR_ERR(tp);
+	}
+
+	/* parse arguments */
+	ret = 0;
+	for (i = 0; i < argc && i < MAX_TRACE_ARGS; i++) {
+		/* Parse argument name */
+		arg = strchr(argv[i], '=');
+		if (arg)
+			*arg++ = '\0';
+		else
+			arg = argv[i];
+
+		if (conflict_field_name(argv[i], tp->args, i)) {
+			pr_info("Argument%d name '%s' conflicts with "
+				"another field.\n", i, argv[i]);
+			ret = -EINVAL;
+			goto error;
+		}
+
+		tp->args[i].name = kstrdup(argv[i], GFP_KERNEL);
+		if (!tp->args[i].name) {
+			pr_info("Failed to allocate argument%d name '%s'.\n",
+				i, argv[i]);
+			ret = -ENOMEM;
+			goto error;
+		}
+
+		/* Parse fetch argument */
+		ret = parse_probe_arg(arg, &tp->args[i].fetch);
+		if (ret) {
+			pr_info("Parse error at argument%d. (%d)\n", i, ret);
+			kfree(tp->args[i].name);
+			goto error;
+		}
+
+		tp->nr_args++;
+	}
+
+	ret = register_trace_uprobe(tp);
+	if (ret)
+		goto error;
+	return 0;
+
+error:
+	free_trace_uprobe(tp);
+	return ret;
+
+fail_address_parse:
+	pr_info("Failed to parse address.\n");
+	return ret;
+}
+
+static void cleanup_all_probes(void)
+{
+	struct trace_uprobe *tp;
+
+	mutex_lock(&uprobe_lock);
+	/* TODO: Use batch unregistration */
+	while (!list_empty(&uprobe_list)) {
+		tp = list_entry(uprobe_list.next, struct trace_uprobe, list);
+		unregister_trace_uprobe(tp);
+		free_trace_uprobe(tp);
+	}
+	mutex_unlock(&uprobe_lock);
+}
+
+
+/* Probes listing interfaces */
+static void *probes_seq_start(struct seq_file *m, loff_t *pos)
+{
+	mutex_lock(&uprobe_lock);
+	return seq_list_start(&uprobe_list, *pos);
+}
+
+static void *probes_seq_next(struct seq_file *m, void *v, loff_t *pos)
+{
+	return seq_list_next(v, &uprobe_list, pos);
+}
+
+static void probes_seq_stop(struct seq_file *m, void *v)
+{
+	mutex_unlock(&uprobe_lock);
+}
+
+static int probes_seq_show(struct seq_file *m, void *v)
+{
+	struct trace_uprobe *tp = v;
+	int i, ret;
+	char buf[MAX_ARGSTR_LEN + 1];
+
+	seq_printf(m, "%c", 'p');
+	seq_printf(m, ":%s/%s", tp->call.system, tp->call.name);
+
+	seq_printf(m, " %d:0x%p", tp->up.pid, (void *)tp->up.vaddr);
+
+	for (i = 0; i < tp->nr_args; i++) {
+		ret = probe_arg_string(buf, MAX_ARGSTR_LEN, &tp->args[i].fetch);
+		if (ret < 0) {
+			pr_warning("Argument%d decoding error(%d).\n", i, ret);
+			return ret;
+		}
+		seq_printf(m, " %s=%s", tp->args[i].name, buf);
+	}
+	seq_printf(m, "\n");
+	return 0;
+}
+
+static const struct seq_operations probes_seq_op = {
+	.start  = probes_seq_start,
+	.next   = probes_seq_next,
+	.stop   = probes_seq_stop,
+	.show   = probes_seq_show
+};
+
+static int probes_open(struct inode *inode, struct file *file)
+{
+	if ((file->f_mode & FMODE_WRITE) &&
+	    (file->f_flags & O_TRUNC))
+		cleanup_all_probes();
+
+	return seq_open(file, &probes_seq_op);
+}
+
+static int command_trace_uprobe(const char *buf)
+{
+	char **argv;
+	int argc = 0, ret = 0;
+
+	argv = argv_split(GFP_KERNEL, buf, &argc);
+	if (!argv)
+		return -ENOMEM;
+
+	if (argc)
+		ret = create_trace_uprobe(argc, argv);
+
+	argv_free(argv);
+	return ret;
+}
+
+#define WRITE_BUFSIZE 128
+
+static ssize_t probes_write(struct file *file, const char __user *buffer,
+			    size_t count, loff_t *ppos)
+{
+	char *kbuf, *tmp;
+	int ret;
+	size_t done;
+	size_t size;
+
+	kbuf = kmalloc(WRITE_BUFSIZE, GFP_KERNEL);
+	if (!kbuf)
+		return -ENOMEM;
+
+	ret = done = 0;
+	while (done < count) {
+		size = count - done;
+		if (size >= WRITE_BUFSIZE)
+			size = WRITE_BUFSIZE - 1;
+		if (copy_from_user(kbuf, buffer + done, size)) {
+			ret = -EFAULT;
+			goto out;
+		}
+		kbuf[size] = '\0';
+		tmp = strchr(kbuf, '\n');
+		if (tmp) {
+			*tmp = '\0';
+			size = tmp - kbuf + 1;
+		} else if (done + size < count) {
+			pr_warning("Line length is too long: "
+				   "Should be less than %d.", WRITE_BUFSIZE);
+			ret = -EINVAL;
+			goto out;
+		}
+		done += size;
+		/* Remove comments */
+		tmp = strchr(kbuf, '#');
+		if (tmp)
+			*tmp = '\0';
+
+		ret = command_trace_uprobe(kbuf);
+		if (ret)
+			goto out;
+	}
+	ret = done;
+out:
+	kfree(kbuf);
+	return ret;
+}
+
+static const struct file_operations uprobe_events_ops = {
+	.owner          = THIS_MODULE,
+	.open           = probes_open,
+	.read           = seq_read,
+	.llseek         = seq_lseek,
+	.release        = seq_release,
+	.write		= probes_write,
+};
+
+/* Probes profiling interfaces */
+static int probes_profile_seq_show(struct seq_file *m, void *v)
+{
+	struct trace_uprobe *tp = v;
+
+	seq_printf(m, "  %d %-44s %15lu\n",tp->up.pid, tp->call.name, tp->nhit);
+	return 0;
+}
+
+static const struct seq_operations profile_seq_op = {
+	.start  = probes_seq_start,
+	.next   = probes_seq_next,
+	.stop   = probes_seq_stop,
+	.show   = probes_profile_seq_show
+};
+
+static int profile_open(struct inode *inode, struct file *file)
+{
+	return seq_open(file, &profile_seq_op);
+}
+
+static const struct file_operations uprobe_profile_ops = {
+	.owner          = THIS_MODULE,
+	.open           = profile_open,
+	.read           = seq_read,
+	.llseek         = seq_lseek,
+	.release        = seq_release,
+};
+
+/* Uprobe handler */
+static void uprobe_trace_func(struct uprobe *up, struct pt_regs *regs)
+{
+	struct trace_uprobe *tp = container_of(up, struct trace_uprobe, up);
+	struct uprobe_trace_entry *entry;
+	struct ring_buffer_event *event;
+	struct ring_buffer *buffer;
+	int size, i, pc;
+	unsigned long irq_flags;
+	struct ftrace_event_call *call = &tp->call;
+
+	tp->nhit++;
+
+	local_save_flags(irq_flags);
+	pc = preempt_count();
+
+	size = SIZEOF_UPROBE_TRACE_ENTRY(tp->nr_args);
+
+	event = trace_current_buffer_lock_reserve(&buffer, call->id, size,
+						  irq_flags, pc);
+	if (!event)
+		return;
+
+	entry = ring_buffer_event_data(event);
+	entry->nargs = tp->nr_args;
+	entry->ip = (unsigned long)up->vaddr;
+	for (i = 0; i < tp->nr_args; i++)
+		entry->args[i] = call_fetch(&tp->args[i].fetch, regs);
+
+	if (!filter_current_check_discard(buffer, call, entry, event))
+		trace_nowake_buffer_unlock_commit(buffer, event, irq_flags, pc);
+}
+
+/* Event entry printers */
+enum print_line_t
+print_uprobe_event(struct trace_iterator *iter, int flags)
+{
+	struct uprobe_trace_entry *field;
+	struct trace_seq *s = &iter->seq;
+	struct trace_event *event;
+	struct trace_uprobe *tp;
+	int i;
+
+	field = (struct uprobe_trace_entry *)iter->ent;
+	event = ftrace_find_event(field->ent.type);
+	tp = container_of(event, struct trace_uprobe, event);
+
+	if (!trace_seq_printf(s, "%s: (", tp->call.name))
+		goto partial;
+
+	if (!seq_print_ip_sym(s, field->ip, flags | TRACE_ITER_SYM_OFFSET))
+		goto partial;
+
+	if (!trace_seq_puts(s, ")"))
+		goto partial;
+
+	for (i = 0; i < field->nargs; i++)
+		if (!trace_seq_printf(s, " %s=%lx",
+				      tp->args[i].name, field->args[i]))
+			goto partial;
+
+	if (!trace_seq_puts(s, "\n"))
+		goto partial;
+
+	return TRACE_TYPE_HANDLED;
+partial:
+	return TRACE_TYPE_PARTIAL_LINE;
+}
+
+
+static int probe_event_enable(struct ftrace_event_call *call)
+{
+	int ret = 0;
+	struct trace_uprobe *tp = (struct trace_uprobe *)call->data;
+
+	if (!(tp->flags & UPROBE_ENABLED)) {
+		ret = register_uprobe(&tp->up);
+		if (!ret)
+			tp->flags |= (UPROBE_ENABLED | TP_FLAG_TRACE);
+	}
+	return ret;
+}
+
+static void probe_event_disable(struct ftrace_event_call *call)
+{
+	struct trace_uprobe *tp = (struct trace_uprobe *)call->data;
+
+	if (tp->flags & UPROBE_ENABLED) {
+		unregister_uprobe(&tp->up);
+		tp->flags &= ~(UPROBE_ENABLED | TP_FLAG_TRACE);
+	}
+}
+
+static int probe_event_raw_init(struct ftrace_event_call *event_call)
+{
+	INIT_LIST_HEAD(&event_call->fields);
+
+	return 0;
+}
+
+#undef DEFINE_FIELD
+#define DEFINE_FIELD(type, item, name, is_signed)			\
+	do {								\
+		ret = trace_define_field(event_call, #type, name,	\
+					 offsetof(typeof(field), item),	\
+					 sizeof(field.item), is_signed, \
+					 FILTER_OTHER);			\
+		if (ret)						\
+			return ret;					\
+	} while (0)

;

kernel/trace/trace_uprobe.c new file +39 -0

+	return 0;
+}
+
+static int __set_print_fmt(struct trace_uprobe *tp, char *buf, int len)
+{
+	int i;
+	int pos = 0;
+
+	const char *fmt, *arg;
+
+	fmt = "(%lx)";
+	arg = "REC->" FIELD_STRING_IP;
+
+	/* When len=0, we just calculate the needed length */
+#define LEN_OR_ZERO (len ? len - pos : 0)
+
+	pos += snprintf(buf + pos, LEN_OR_ZERO, "\"%s", fmt);
+	for (i = 0; i < tp->nr_args; i++)
+		pos += snprintf(buf + pos, LEN_OR_ZERO, " %s=%%lx",
+				tp->args[i].name);
+
+	pos += snprintf(buf + pos, LEN_OR_ZERO, "\", %s", arg);
+	for (i = 0; i < tp->nr_args; i++)
+		pos += snprintf(buf + pos, LEN_OR_ZERO, ", REC->%s",
+				tp->args[i].name);
+
+#undef LEN_OR_ZERO
+
+	/* return the length of print_fmt */
+	return pos;
+}
+
+static int set_print_fmt(struct trace_uprobe *tp)
+{
+	int len;
+	char *print_fmt;
+
+	/* First: called with 0 length to calculate the needed length */
+	len = __set_print_fmt(tp, NULL, 0);

kernel/trace/trace_uprobe.c new file +144 -1

+
+#ifdef CONFIG_PERF_EVENTS
+
+/* Uprobe profile handler */
+static void uprobe_perf_func(struct uprobe *up,
+					 struct pt_regs *regs)
+{
+	struct trace_uprobe *tp = container_of(up, struct trace_uprobe, up);
+	struct ftrace_event_call *call = &tp->call;
+	struct uprobe_trace_entry *entry;
+	int size, __size, i;
+	unsigned long irq_flags;
+	int rctx;
+
+	__size = SIZEOF_UPROBE_TRACE_ENTRY(tp->nr_args);
+	size = ALIGN(__size + sizeof(u32), sizeof(u64));
+	size -= sizeof(u32);
+	if (WARN_ONCE(size > PERF_MAX_TRACE_SIZE,
+		     "profile buffer not large enough"))
+		return;
+
+	entry = perf_trace_buf_prepare(size, call->id, &rctx, &irq_flags);
+	if (!entry)
+		return;
+
+	entry->nargs = tp->nr_args;
+	entry->ip = (unsigned long)up->vaddr;
+	for (i = 0; i < tp->nr_args; i++)
+		entry->args[i] = call_fetch(&tp->args[i].fetch, regs);
+
+	perf_trace_buf_submit(entry, size, rctx, entry->ip, 1, irq_flags, regs);
+}
+
+static int probe_perf_enable(struct ftrace_event_call *call)
+{
+	int ret = 0;
+	struct trace_uprobe *tp = (struct trace_uprobe *)call->data;
+
+	if (!(tp->flags & UPROBE_ENABLED)) {
+		ret = register_uprobe(&tp->up);
+		if (!ret) 
+			tp->flags |= (UPROBE_ENABLED | TP_FLAG_PROFILE);
+	}
+	return ret;
+}
+
+static void probe_perf_disable(struct ftrace_event_call *call)
+{
+	struct trace_uprobe *tp = (struct trace_uprobe *)call->data;
+
+	if (tp->flags & UPROBE_ENABLED) {
+		unregister_uprobe(&tp->up);
+		tp->flags &= ~(UPROBE_ENABLED | TP_FLAG_PROFILE);
+	}
+}
+#endif	/* CONFIG_PERF_EVENTS */
+
+
+static 
+void uprobe_dispatcher(struct uprobe *up, struct pt_regs *regs)
+{
+	struct trace_uprobe *tp = container_of(up, struct trace_uprobe, up);
+
+	if (tp->flags & TP_FLAG_TRACE)
+		uprobe_trace_func(up, regs);
+#ifdef CONFIG_PERF_EVENTS
+	if (tp->flags & TP_FLAG_PROFILE)
+		uprobe_perf_func(up, regs);
+#endif
+}
+
+
+static int register_uprobe_event(struct trace_uprobe *tp)
+{
+	struct ftrace_event_call *call = &tp->call;
+	int ret;
+
+	/* Initialize ftrace_event_call */
+	tp->event.trace = print_uprobe_event;
+	call->raw_init = probe_event_raw_init;
+	call->define_fields = uprobe_event_define_fields;
+	if (set_print_fmt(tp) < 0)
+		return -ENOMEM;
+
+	call->event = &tp->event;
+	call->id = register_ftrace_event(&tp->event);
+	if (!call->id) {
+		kfree(call->print_fmt);
+		return -ENODEV;
+	}
+
+	call->enabled = 0;
+	call->regfunc = probe_event_enable;
+	call->unregfunc = probe_event_disable;
+
+#ifdef CONFIG_PERF_EVENTS
+	call->perf_event_enable = probe_perf_enable;
+	call->perf_event_disable = probe_perf_disable;
+#endif
+	call->data = tp;
+	ret = trace_add_event_call(call);
+	if (ret) {
+		pr_info("Failed to register uprobe event: %s\n", call->name);
+		kfree(call->print_fmt);
+		unregister_ftrace_event(&tp->event);
+	}
+	return ret;
+}
+
+static void unregister_uprobe_event(struct trace_uprobe *tp)
+{
+	/* tp->event is unregistered in trace_remove_event_call() */
+	trace_remove_event_call(&tp->call);
+	kfree(tp->call.print_fmt);
+}
+
+/* Make a debugfs interface for controling probe points */
+static __init int init_uprobe_trace(void)
+{
+	struct dentry *d_tracer;
+	struct dentry *entry;
+
+	d_tracer = tracing_init_dentry();
+	if (!d_tracer)
+		return 0;
+
+	entry = debugfs_create_file("uprobe_events", 0644, d_tracer,
+				    NULL, &uprobe_events_ops);
+
+	/* Event list interface */
+	if (!entry)
+		pr_warning("Could not create debugfs "
+			   "'uprobe_events' entry\n");
+
+	/* Profile interface */
+	entry = debugfs_create_file("uprobe_profile", 0444, d_tracer,
+				    NULL, &uprobe_profile_ops);
+
+	if (!entry)
+		pr_warning("Could not create debugfs "
+			   "'uprobe_profile' entry\n");
+	return 0;
+}
+fs_initcall(init_uprobe_trace);
--

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/