On 7/30/20 5:04 PM, Steven Rostedt wrote: > On Thu, 30 Jul 2020 16:29:12 +0200 > peter enderborg <peter.enderb...@sony.com> wrote: > >> +#undef TRACE_SYSTEM >> +#define TRACE_SYSTEM avc >> + >> +#if !defined(_TRACE_AVC_H) || defined(TRACE_HEADER_MULTI_READ) >> +#define _TRACE_AVC_H >> + >> +#include <linux/tracepoint.h> >> +TRACE_EVENT(avc_data, >> + TP_PROTO(u32 requested, >> + u32 denied, >> + u32 audited, >> + int result, >> + const char *msg >> + ), >> + >> + TP_ARGS(requested, denied, audited, result,msg), >> + >> + TP_STRUCT__entry( >> + __field(u32, requested) >> + __field(u32, denied) >> + __field(u32, audited) >> + __field(int, result) >> + __array(char, msg, 255) > You want to use __string() here, otherwise you are wasting a lot of > buffer space. > > __string( msg, msg) It should be a full structure with a lot of sub strings. But that make is even more relevant. > >> + ), >> + >> + TP_fast_assign( >> + __entry->requested = requested; >> + __entry->denied = denied; >> + __entry->audited = audited; >> + __entry->result = result; >> + memcpy(__entry->msg, msg, 255); > Not to mention, the above is a bug. As the msg being passed in, is > highly unlikely to be 255 bytes. You just leaked all that memory after > the sting to user space. > > Where you want here: > > __assign_str( msg, msg );
Directly in to the code. Was more in to get in to discussion on how complex we should have the trace data. There is a lot of fields. Not all is always present. Is there any good way to handle that? Like "something= somethingelse=42" or "something=nil somthingelse=42" > > -- Steve > > > >> + ), >> + >> + TP_printk("requested=0x%x denied=%d audited=%d result=%d >> msg=%s", >> + __entry->requested, __entry->denied, __entry->audited, >> __entry->result, __entry->msg >> + )