linux-kernel - Re: [PATCH] tracing/uprobes: Support ftrace_event

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:	Thu, 20 Jun 2013 18:43:55 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	"zhangwei(Jovi)" <jovi.zhangwei@...wei.com>
Cc:	Steven Rostedt <rostedt@...dmis.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Ingo Molnar <mingo@...hat.com>,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	Srikar Dronamraju <srikar@...ux.vnet.ibm.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] tracing/uprobes: Support ftrace_event_file base
	multibuffer

Hi,

Now that we finished the discussion of the similar code in kprobes,
let me summarize.

On 06/14, Oleg Nesterov wrote:
>
> On 06/14, zhangwei(Jovi) wrote:
> >
> > +		rcu_assign_pointer(tu->files, new);
> > +		tu->flags |= TP_FLAG_TRACE;
> > +
> > +		if (old) {
> > +			/* Make sure the probe is done with old files */
> > +			synchronize_sched();
> > +			kfree(old);
> > +		}
> > +	} else
> > +		tu->flags |= TP_FLAG_PROFILE;
>
> So it can set both TP_FLAG_TRACE and TP_FLAG_PROFILE, yes?
>
> If yes, this is not right. Until we change the pre-filtering at least.
> Currently TP_FLAG_TRACE/TP_FLAG_PROFILE are mutually exclusive.

Yes. So please update this patch so that TP_FLAG_PROFILE can't be
set if TP_FLAG_TRACE is set and vice versa.

Once again, this limitation is artificial. But it was always here,
and we need more changes to remove it. I'll try to do this later.
(but if you want to play with this code - welcome ;)

Don't add the mutex, and do not use array-of-pointers (I hope you
noticed the recent discussion).

Locking. Oh, OK. Let it be rcu for now (but please do not forget
that you need rcu_read_lock, uprobe handlers run in the sleepable
context). This is sub-optimal, but this is just another indication
that uprobes API is not perfect, we can't use uprobe->register_sem.

Also. When I was reading trace_kprobes.c I notice the new (and imho
useful) feature, soft disable/enable. Perhaps you can make a 2nd
patch which adds the FTRACE_EVENT_FL_SOFT_DISABLED_BIT check?

Oleg.

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