linux-kernel - RE: [tip:perf/core] perf: Add pmu specific data for perf task context

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Wed, 9 Dec 2015 14:59:21 +0000
From:	"Liang, Kan" <kan.liang@...el.com>
To:	Peter Zijlstra <peterz@...radead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"hpa@...or.com" <hpa@...or.com>,
	"torvalds@...ux-foundation.org" <torvalds@...ux-foundation.org>,
	"acme@...nel.org" <acme@...nel.org>,
	"mingo@...nel.org" <mingo@...nel.org>,
	"paulus@...ba.org" <paulus@...ba.org>,
	"tglx@...utronix.de" <tglx@...utronix.de>
CC:	"linux-tip-commits@...r.kernel.org" 
	<linux-tip-commits@...r.kernel.org>
Subject: RE: [tip:perf/core] perf: Add pmu specific data for perf task
 context


> 
> On Wed, Feb 18, 2015 at 09:15:06AM -0800, tip-bot for Yan, Zheng wrote:
> > +find_get_context(struct pmu *pmu, struct task_struct *task,
> > +		struct perf_event *event)
> >  {
> >  	struct perf_event_context *ctx, *clone_ctx = NULL;
> >  	struct perf_cpu_context *cpuctx;
> > +	void *task_ctx_data = NULL;
> >  	unsigned long flags;
> >  	int ctxn, err;
> > +	int cpu = event->cpu;
> >
> >  	if (!task) {
> >  		/* Must be root to operate on a CPU event: */ @@ -
> 3342,11 +3354,24
> > @@ find_get_context(struct pmu *pmu, struct task_struct *task, int cpu)
> >  	if (ctxn < 0)
> >  		goto errout;
> >
> > +	if (event->attach_state & PERF_ATTACH_TASK_DATA) {
> > +		task_ctx_data = kzalloc(pmu->task_ctx_size, GFP_KERNEL);
> > +		if (!task_ctx_data) {
> > +			err = -ENOMEM;
> > +			goto errout;
> > +		}
> > +	}
> > +
> >  retry:
> >  	ctx = perf_lock_task_context(task, ctxn, &flags);
> >  	if (ctx) {
> >  		clone_ctx = unclone_ctx(ctx);
> >  		++ctx->pin_count;
> > +
> > +		if (task_ctx_data && !ctx->task_ctx_data) {
> > +			ctx->task_ctx_data = task_ctx_data;
> > +			task_ctx_data = NULL;
> > +		}
> >  		raw_spin_unlock_irqrestore(&ctx->lock, flags);
> >
> >  		if (clone_ctx)
> > @@ -3357,6 +3382,11 @@ retry:
> >  		if (!ctx)
> >  			goto errout;
> >
> > +		if (task_ctx_data) {
> > +			ctx->task_ctx_data = task_ctx_data;
> > +			task_ctx_data = NULL;
> > +		}
> > +
> >  		err = 0;
> >  		mutex_lock(&task->perf_event_mutex);
> >  		/*
> > @@ -3383,9 +3413,11 @@ retry:
> >  		}
> >  	}
> >
> > +	kfree(task_ctx_data);
> >  	return ctx;
> >
> >  errout:
> > +	kfree(task_ctx_data);
> >  	return ERR_PTR(err);
> >  }
> 
> 
> diff --git a/kernel/events/core.c b/kernel/events/core.c index
> 36babfd..97aa610 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -3508,11 +3515,6 @@ retry:
>  		if (!ctx)
>  			goto errout;
> 
> -		if (task_ctx_data) {
> -			ctx->task_ctx_data = task_ctx_data;
> -			task_ctx_data = NULL;
> -		}
> -
>  		err = 0;
>  		mutex_lock(&task->perf_event_mutex);
>  		/*
> @@ -3526,6 +3528,10 @@ retry:
>  		else {
>  			get_ctx(ctx);
>  			++ctx->pin_count;
> +			if (task_ctx_data) {
> +				ctx->task_ctx_data = task_ctx_data;
> +				task_ctx_data = NULL;
> +			}
>  			rcu_assign_pointer(task->perf_event_ctxp[ctxn],
> ctx);
>  		}
>  		mutex_unlock(&task->perf_event_mutex);
> 
> 
> Does that make sense? No point in setting task_ctx_data if we're going to
> free the ctx and try again.

The task_ctx_data will be checked before use. So it wouldn't crash the
system if it's NULL.
The problem is that LBR stack info will not be save/store on context switch
anymore. The user probably get wrong call stack information.
May I know why you want to do that?

Thanks,
Kan


--
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/