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

Tue Oct 4 17:08:57 UTC 2016

On 16-10-03 06:21 PM, Mathieu Desnoyers wrote:
> ----- 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

It makes sense.  I just tested it and it works fine, here's a prototype.

On a development machine, it's easy to enable once and for all by putting

  options lttng-probe-i2c trace_sensitive_data=1

in /etc/modprobe.d/lttng.conf for example.

Here I have put the knob in the i2c module itself, which means that if we have
this situation in other modules, we'll have to copy the code that defines the
parameter.  That gives us a module-level granularity, which can be an upside
(more control over which modules collect sensitive data) or a downside (more
complex, need to pass the option too all modules).

I assume it would be possible to put the parameter in the lttng-tracer module,
on which all other modules depend, in order to have an all-or-nothing
approach.

One cool thing about module parameters is that they can be modified dynamically
by writing to sysfs (/sys/module/lttng_probe_i2c/parameters/trace_sensitive_data).
So you can change the setting mid-trace, I tried it and it works.

If the gathering of sensitive info is off, the sequence of bytes is just empty.
Since you have the length of the message as a different field, the length of the
sequence does not match the former.  The trace viewer can probably use this
mismatch as a sign that the data was omitted, and that it might not have been an
actual empty message.


>From 69aa1c3d73b5b0dbf031b90c6e38d4b2a0ad8699 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi at ericsson.com>
Date: Tue, 4 Oct 2016 11:38:19 -0400
Subject: [PATCH] Provide knob to control tracing of i2c buffer contents

---
 instrumentation/events/lttng-module/i2c.h | 6 ++++--
 probes/lttng-probe-i2c.c                  | 7 +++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/instrumentation/events/lttng-module/i2c.h b/instrumentation/events/lttng-module/i2c.h
index 68ce80d..1ede276 100644
--- a/instrumentation/events/lttng-module/i2c.h
+++ b/instrumentation/events/lttng-module/i2c.h
@@ -26,7 +26,8 @@ LTTNG_TRACEPOINT_EVENT(i2c_write,
 		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)
+		ctf_sequence_hex(__u8, buf, trace_sensitive_data ? msg->buf : NULL,
+				 __u16, trace_sensitive_data ? msg->len : 0)
 	)
 )

@@ -63,7 +64,8 @@ LTTNG_TRACEPOINT_EVENT(i2c_reply,
 		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)
+		ctf_sequence_hex(__u8, buf, trace_sensitive_data ? msg->buf : NULL,
+				 __u16, trace_sensitive_data ? msg->len : 0)
 	)
 )

diff --git a/probes/lttng-probe-i2c.c b/probes/lttng-probe-i2c.c
index 94ebdc0..fd30a73 100644
--- a/probes/lttng-probe-i2c.c
+++ b/probes/lttng-probe-i2c.c
@@ -21,6 +21,7 @@
  */

 #include <linux/module.h>
+#include <linux/moduleparam.h>
 #include <lttng-tracer.h>

 /*
@@ -36,6 +37,12 @@
 #define CREATE_TRACE_POINTS
 #define TRACE_INCLUDE_PATH instrumentation/events/lttng-module

+static int trace_sensitive_data = 0;
+module_param(trace_sensitive_data, int, 0644);
+MODULE_PARM_DESC(trace_sensitive_data, "Whether to trace possibly sensitive "
+				       "data (i2c buffer contents) or not (1 or "
+				       "0, default: 0).");
+
 #include <instrumentation/events/lttng-module/i2c.h>

 MODULE_LICENSE("GPL and additional rights");
-- 
2.10.1



