From mboxrd@z Thu Jan 1 00:00:00 1970 From: mathieu.desnoyers@efficios.com (Mathieu Desnoyers) Date: Tue, 4 Oct 2016 21:21:12 +0000 (UTC) Subject: [lttng-dev] [PATCH lttng-modules] Add support for i2c tracepoints In-Reply-To: References: <20161003182115.30428-1-simon.marchi@ericsson.com> <93496556.44682.1475533300165.JavaMail.zimbra@efficios.com> <1386914197.45790.1475602673694.JavaMail.zimbra@efficios.com> Message-ID: <1360149668.46148.1475616072920.JavaMail.zimbra@efficios.com> ----- On Oct 4, 2016, at 3:48 PM, Simon Marchi simon.marchi at ericsson.com wrote: > On 16-10-04 01:37 PM, Mathieu Desnoyers wrote: >> Perhaps we could rename the option to "trace_sensitive_payload" ? > > Of all those: > > - trace_sensitive_data > - trace_sensitive_payload > - allow_sensitive_payload_extraction > - extract_sensitive_payload > > I prefer "extract_sensitive_payload". But it's your call, just tell me which > one you prefer. Let's go for "extract_sensitive_payload". > >> I think we should keep it at the probe level, like you do in your prototype. >> >>> >>> >>> From 69aa1c3d73b5b0dbf031b90c6e38d4b2a0ad8699 Mon Sep 17 00:00:00 2001 >>> From: Simon Marchi >>> 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) >> >> Since "trace_sensitive_data" can be changed at runtime concurrently >> with tracing, we should load it with LOAD_ONCE(), keep it in a local >> variable (TRACEPOINT_EVENT_CODE, TP_locvar and TP_code), and use that >> in as argument. Otherwise the compiler can perform two fetch of that >> variable, and its value may change in between, thus leading to a >> NULL pointer dereference (OOPS). > > Ahh good catch. I'll look up those things you mentioned, but do you > have a usage pattern in mind that we could use here? Where would the > local variable be? I know of gcc's statement expression, we could use > that, but do you have something else in mind? See http://lttng.org/docs/#doc-lttng-tracepoint-event-code Note that there are slight changes to TP_code() in 2.8. See source for details. > >>> ) >>> ) >>> >>> @@ -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) >>> ) >>> ) >>> >> >> Same here. >> >> Can you also fold it with your i2c patch ? > > I'll send a v2 of the original patch once it's clear how to do the load once > thingy. Allright, thanks! Mathieu > > Thanks! > > Simon -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com