From mboxrd@z Thu Jan 1 00:00:00 1970 From: mathieu.desnoyers@efficios.com (Mathieu Desnoyers) Date: Mon, 3 Oct 2016 22:21:40 +0000 (UTC) Subject: [lttng-dev] [PATCH lttng-modules] Add support for i2c tracepoints In-Reply-To: <20161003182115.30428-1-simon.marchi@ericsson.com> References: <20161003182115.30428-1-simon.marchi@ericsson.com> Message-ID: <93496556.44682.1475533300165.JavaMail.zimbra@efficios.com> ----- 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 > --- > 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 > + > +#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 > 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 > + * > + * 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 > +#include > + > +/* > + * Create the tracepoint static inlines from the kernel to validate that our > + * trace event macros match the kernel we run on. > + */ > +#include > + > +/* > + * Create LTTng tracepoint probes. > + */ > +#define LTTNG_PACKAGE_BUILD > +#define CREATE_TRACE_POINTS > +#define TRACE_INCLUDE_PATH instrumentation/events/lttng-module > + > +#include > + > +MODULE_LICENSE("GPL and additional rights"); > +MODULE_AUTHOR("Mathieu Desnoyers "); > +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