linux-kernel - Re: [PATCH v1 6/9] uretprobes: Return probe exit, invoke handlers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130407105308.GA2899@linux.vnet.ibm.com>
Date:	Sun, 7 Apr 2013 16:23:08 +0530
From:	Srikar Dronamraju <srikar@...ux.vnet.ibm.com>
To:	Anton Arapov <anton@...hat.com>
Cc:	Oleg Nesterov <oleg@...hat.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Josh Stone <jistone@...hat.com>,
	Frank Eigler <fche@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...e.hu>,
	Ananth N Mavinakayanahalli <ananth@...ibm.com>,
	adrian.m.negreanu@...el.com, Torsten.Polle@....de
Subject: Re: [PATCH v1 6/9] uretprobes: Return probe exit, invoke handlers

* Anton Arapov <anton@...hat.com> [2013-04-03 18:00:36]:

> Uretprobe handlers are invoked when the trampoline is hit, on completion the
> trampoline is replaced with the saved return address and the uretprobe instance
> deleted.
> 
> v1 changes:
> * pass bp_vaddr to ret_handler()
> * simplify handle_uretprobe()
> 
> RFCv6 changes:
> * rework handle_uretprobe()
> 
> RFCv5 changes:
> * switch to simply linked list ->return_uprobes
> * rework handle_uretprobe()
> 
> RFCv4 changes:
> * check, whether utask is not NULL in handle_uretprobe()
> * get rid of area->rp_trampoline_vaddr
> * minor handle_uretprobe() fixups
> 
> RFCv3 changes:
> * protected uprobe with refcounter. See put_uprobe() in handle_uretprobe()
>   that reflects increment in prepare_uretprobe()
> 
> RFCv2 changes:
> * get rid of ->return_consumers member from struct uprobe, introduce
>   ret_handler() in consumer instead
> 
> Signed-off-by: Anton Arapov <anton@...hat.com>
> ---
>  kernel/events/uprobes.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 59 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 08ecfff..d129c1d 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -1609,6 +1609,57 @@ static void handler_chain(struct uprobe *uprobe, struct pt_regs *regs)
>  	up_read(&uprobe->register_rwsem);
>  }
> 
> +static void
> +handler_uretprobe_chain(struct return_instance *ri, struct pt_regs *regs)

> +{
> +	struct uprobe *uprobe = ri->uprobe;
> +	struct uprobe_consumer *uc;
> +
> +	down_read(&uprobe->register_rwsem);
> +	for (uc = uprobe->consumers; uc; uc = uc->next) {
> +		if (uc->ret_handler)
> +			uc->ret_handler(uc, ri->func, regs);
> +	}
> +	up_read(&uprobe->register_rwsem);
> +}
> +
> +static bool handler_uretprobe(struct pt_regs *regs)

Nit: can this be renamed to handle_trampoline

> +{
> +	struct uprobe_task *utask;
> +	struct return_instance *ri, *tmp;
> +	bool chained;
> +
> +	utask = current->utask;
> +	if (!utask)
> +		return false;
> +
> +	ri = utask->return_instances;
> +	if (!ri)
> +		return false;
> +
> +	instruction_pointer_set(regs, ri->orig_ret_vaddr);

Should we a check here before using top most ri.
What if the application had done a longjmp and the trampoline he hit
corresponds to something thats below in the stack?

Not sure if this what you meant by leaking return instances in your next
patch.


> +
> +	for (;;) {
> +		handler_uretprobe_chain(ri, regs);
> +
> +		chained = ri->chained;
> +		put_uprobe(ri->uprobe);
> +
> +		tmp = ri;
> +		ri = ri->next;
> +		kfree(tmp);
> +
> +		if (!chained)
> +			break;
> +
> +		BUG_ON(!ri);
> +	}
> +
> +	utask->return_instances = ri;
> +
> +	return true;
> +}
> +
>  /*
>   * Run handler and ask thread to singlestep.
>   * Ensure all non-fatal signals cannot interrupt thread while it singlesteps.
> @@ -1620,8 +1671,15 @@ static void handle_swbp(struct pt_regs *regs)
>  	int uninitialized_var(is_swbp);
> 
>  	bp_vaddr = uprobe_get_swbp_addr(regs);
> -	uprobe = find_active_uprobe(bp_vaddr, &is_swbp);
> +	if (bp_vaddr == get_trampoline_vaddr()) {
> +		if (handler_uretprobe(regs))
> +			return;
> 
> +		pr_warn("uprobe: unable to handle uretprobe pid/tgid=%d/%d\n",
> +						current->pid, current->tgid);
> +	}
> +
> +	uprobe = find_active_uprobe(bp_vaddr, &is_swbp);
>  	if (!uprobe) {
>  		if (is_swbp > 0) {
>  			/* No matching uprobe; signal SIGTRAP. */
> -- 
> 1.8.1.4
> 

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