[lttng-dev] [PATCH lttng-modules] Add support for i2c tracepoints

Mon Oct 3 22:21:40 UTC 2016

----- On Oct 3, 2016, at 2:21 PM, Simon Marchi simon.marchi at ericsson.com wrote:

> This patch teaches lttng-modules about the i2c tracepoints in the Linux
> kernel.
> 
> It contains the following tracepoints:
> 
>  * i2c_write
>  * i2c_read
>  * i2c_reply
>  * i2c_result
> 
> I translated the fields and assignments from the kernel's
> include/trace/events/i2c.h as well as I could.  I also tried building
> this module against a kernel without CONFIG_I2C, and it built fine (the
> required types are unconditionally defined).  So I don't think any "#if
> CONFIG_I2C" or similar are required.

Hi Simon,

The global approach taken in this patch looks good. However, it raises the
question of sensitive information. I'm pretty sure i2c buffers may contain
keyboard keystrokes, which is sensitive info (passwords). It's the same
with read/write system call payloads, network application-level payload,
and HID keystrokes. Back in the days of lttng 0.x, I made sure it would
not allow users to gather a trace with sensitive information by mistake.
You really had to clearly enable gathering of such sensitive information.

One possibility in lttng-modules 2.x would be to introduce a module
parameter for each lttng-modules probe that would specify whether
sensitive information should be traced. I could be specified by the
system administrator at module load time. The default would be that
no sensitive information is gathered.

I think we should *not* make this a per-session/dynamic option, because
it's really something that the machine administrator should decide on
a machine-by-machine basis. In production environments, this would be
expected to disallow gathering sensitive info (default).

In specific use-cases where security is no concern (e.g. embedded board
during development), this could be explicitly enabled by the developer
with root access by passing the module flag at load time.

What do you think ?

Thanks,

Mathieu

> 
> Signed-off-by: Simon Marchi <simon.marchi at ericsson.com>
> ---
> instrumentation/events/lttng-module/i2c.h | 89 +++++++++++++++++++++++++++++++
> probes/Kbuild                             |  1 +
> probes/lttng-probe-i2c.c                  | 47 ++++++++++++++++
> 3 files changed, 137 insertions(+)
> create mode 100644 instrumentation/events/lttng-module/i2c.h
> create mode 100644 probes/lttng-probe-i2c.c
> 
> diff --git a/instrumentation/events/lttng-module/i2c.h
> b/instrumentation/events/lttng-module/i2c.h
> new file mode 100644
> index 0000000..68ce80d
> --- /dev/null
> +++ b/instrumentation/events/lttng-module/i2c.h
> @@ -0,0 +1,89 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM i2c
> +
> +#if !defined(LTTNG_TRACE_I2C_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define LTTNG_TRACE_I2C_H
> +
> +#include <probes/lttng-tracepoint-event.h>
> +
> +#ifndef _TRACE_I2C_DEF_
> +#define _TRACE_I2C_DEF_
> +
> +#endif /* _TRACE_I2C_DEF_ */
> +
> +/*
> + * __i2c_transfer() write request
> + */
> +LTTNG_TRACEPOINT_EVENT(i2c_write,
> +
> +	TP_PROTO(const struct i2c_adapter *adap, const struct i2c_msg *msg, int num),
> +
> +	TP_ARGS(adap, msg, num),
> +
> +	TP_FIELDS(
> +		ctf_integer(int, adapter_nr, adap->nr)
> +		ctf_integer(__u16, msg_nr, num)
> +		ctf_integer(__u16, addr, msg->addr)
> +		ctf_integer(__u16, flags, msg->flags)
> +		ctf_integer(__u16, len, msg->len)
> +		ctf_sequence_hex(__u8, buf, msg->buf, __u16, msg->len)
> +	)
> +)
> +
> +/*
> + * __i2c_transfer() read request
> + */
> +LTTNG_TRACEPOINT_EVENT(i2c_read,
> +
> +	TP_PROTO(const struct i2c_adapter *adap, const struct i2c_msg *msg, int num),
> +
> +	TP_ARGS(adap, msg, num),
> +
> +	TP_FIELDS(
> +		ctf_integer(int, adapter_nr, adap->nr)
> +		ctf_integer(__u16, msg_nr, num)
> +		ctf_integer(__u16, addr, msg->addr)
> +		ctf_integer(__u16, flags, msg->flags)
> +		ctf_integer(__u16, len, msg->len)
> +	)
> +)
> +
> +/*
> + * __i2c_transfer() read reply
> + */
> +LTTNG_TRACEPOINT_EVENT(i2c_reply,
> +
> +	TP_PROTO(const struct i2c_adapter *adap, const struct i2c_msg *msg, int num),
> +
> +	TP_ARGS(adap, msg, num),
> +
> +	TP_FIELDS(
> +		ctf_integer(int, adapter_nr, adap->nr)
> +		ctf_integer(__u16, msg_nr, num)
> +		ctf_integer(__u16, addr, msg->addr)
> +		ctf_integer(__u16, flags, msg->flags)
> +		ctf_integer(__u16, len, msg->len)
> +		ctf_sequence_hex(__u8, buf, msg->buf, __u16, msg->len)
> +	)
> +)
> +
> +/*
> + * __i2c_transfer() result
> + */
> +LTTNG_TRACEPOINT_EVENT(i2c_result,
> +
> +	TP_PROTO(const struct i2c_adapter *adap, int num, int ret),
> +
> +	TP_ARGS(adap, num, ret),
> +
> +	TP_FIELDS(
> +		ctf_integer(int, adapter_nr, adap->nr)
> +		ctf_integer(__u16, nr_msgs, num)
> +		ctf_integer(__s16, ret, ret)
> +	)
> +)
> +
> +#endif /*  LTTNG_TRACE_I2C_H */
> +
> +/* This part must be outside protection */
> +#include <probes/define_trace.h>
> diff --git a/probes/Kbuild b/probes/Kbuild
> index 8ae9a6b..ca7c0b1 100644
> --- a/probes/Kbuild
> +++ b/probes/Kbuild
> @@ -5,6 +5,7 @@ include $(TOP_LTTNG_MODULES_DIR)/Makefile.ABI.workarounds
> ccflags-y += -I$(TOP_LTTNG_MODULES_DIR)
> 
> obj-$(CONFIG_LTTNG) += lttng-probe-sched.o
> +obj-$(CONFIG_LTTNG) += lttng-probe-i2c.o
> obj-$(CONFIG_LTTNG) += lttng-probe-irq.o
> obj-$(CONFIG_LTTNG) += lttng-probe-timer.o
> obj-$(CONFIG_LTTNG) += lttng-probe-kmem.o
> diff --git a/probes/lttng-probe-i2c.c b/probes/lttng-probe-i2c.c
> new file mode 100644
> index 0000000..94ebdc0
> --- /dev/null
> +++ b/probes/lttng-probe-i2c.c
> @@ -0,0 +1,47 @@
> +/*
> + * probes/lttng-probe-i2c.c
> + *
> + * LTTng i2c probes.
> + *
> + * Copyright (C) 2012 Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; only
> + * version 2.1 of the License.
> + *
> + * This library 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
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include <linux/module.h>
> +#include <lttng-tracer.h>
> +
> +/*
> + * Create the tracepoint static inlines from the kernel to validate that our
> + * trace event macros match the kernel we run on.
> + */
> +#include <trace/events/i2c.h>
> +
> +/*
> + * Create LTTng tracepoint probes.
> + */
> +#define LTTNG_PACKAGE_BUILD
> +#define CREATE_TRACE_POINTS
> +#define TRACE_INCLUDE_PATH instrumentation/events/lttng-module
> +
> +#include <instrumentation/events/lttng-module/i2c.h>
> +
> +MODULE_LICENSE("GPL and additional rights");
> +MODULE_AUTHOR("Mathieu Desnoyers <mathieu.desnoyers at efficios.com>");
> +MODULE_DESCRIPTION("LTTng i2c probes");
> +MODULE_VERSION(__stringify(LTTNG_MODULES_MAJOR_VERSION) "."
> +	__stringify(LTTNG_MODULES_MINOR_VERSION) "."
> +	__stringify(LTTNG_MODULES_PATCHLEVEL_VERSION)
> +	LTTNG_MODULES_EXTRAVERSION);
> --
> 2.9.3
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev at lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com