linux-kernel - Re: [PATCH 3/8] pstore: Add persistent function tracing

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]

Message-ID: <1342553898.10332.9.camel@gandalf.stny.rr.com>
Date:	Tue, 17 Jul 2012 15:38:18 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Anton Vorontsov <anton.vorontsov@...aro.org>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Kees Cook <keescook@...omium.org>,
	Colin Cross <ccross@...roid.com>,
	Tony Luck <tony.luck@...el.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Ingo Molnar <mingo@...hat.com>, Arnd Bergmann <arnd@...db.de>,
	John Stultz <john.stultz@...aro.org>,
	Shuah Khan <shuahkhan@...il.com>, arve@...roid.com,
	Rebecca Schultz Zavin <rebecca@...roid.com>,
	Jesper Juhl <jj@...osbits.net>,
	Randy Dunlap <rdunlap@...otime.net>,
	Stephen Boyd <sboyd@...eaurora.org>,
	Thomas Meyer <thomas@...3r.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Marco Stornelli <marco.stornelli@...il.com>,
	WANG Cong <xiyou.wangcong@...il.com>,
	linux-kernel@...r.kernel.org, devel@...verdev.osuosl.org,
	linaro-kernel@...ts.linaro.org, patches@...aro.org,
	kernel-team@...roid.com
Subject: Re: [PATCH 3/8] pstore: Add persistent function tracing

On Mon, 2012-07-09 at 17:10 -0700, Anton Vorontsov wrote:

> --- /dev/null
> +++ b/fs/pstore/ftrace.c
> @@ -0,0 +1,35 @@
> +/*
> + * Copyright 2012  Google, Inc.
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * 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.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/compiler.h>
> +#include <linux/irqflags.h>
> +#include <linux/percpu.h>
> +#include <linux/smp.h>
> +#include <linux/atomic.h>
> +#include <asm/barrier.h>
> +#include "internal.h"
> +
> +void notrace pstore_ftrace_call(unsigned long ip, unsigned long parent_ip)

BTW, you can make the entire file 'notrace' without adding annotations
by including in the Makefile:

CFLAGS_REMOVE_ftrace.o = -pg


> +{
> +	struct pstore_ftrace_record rec = {};
> +
> +	if (unlikely(oops_in_progress))
> +		return;
> +
> +	rec.ip = ip;
> +	rec.parent_ip = parent_ip;
> +	pstore_ftrace_encode_cpu(&rec, raw_smp_processor_id());
> +	psinfo->write_buf(PSTORE_TYPE_FTRACE, 0, NULL, 0, (void *)&rec,
> +			  sizeof(rec), psinfo);
> +}

BTW, can any of the called functions go into module code that can be
removed? If so, then this is not safe at all. Normal function tracing
can not be synced in a preemptible kernel.

Also, I'm starting to wonder if this should be in its own utility
(separate debugfs?) than hooking directly into ftrace. Then you don't
need to modify ftrace at all and you can do the following:

static struct ftrace_ops trace_ops {
	.func = pstore_ftrace_call;
};

then in your write to debugfs file:

	register_ftrace_function(&trace_ops);

To turn off tracing:

	unregister_ftrace_function(&trace_ops);

Note, it's safe to use if the trace_ops (or anything the callback calls)
is a module. That's because it detects the trace_ops is not kernel core
code and will place a wrapper around it that allows the function tracing
to by synced with module unload. You still need to unregister the
trace_ops before unloading the module, or you can have a crash that way.

-- Steve


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