On 5/28/21 11:42 PM, Steven Rostedt wrote: > On Wed, 26 May 2021 12:18:37 -0400 > Olivier Langlois <olivier@xxxxxxxxxxxxxx> wrote: > >>> If that gets changed, could be also include the personality id and >>> flags here, >>> and maybe also translated the opcode and flags to human readable >>> strings? >>> >> If Jens and Pavel agrees that they would like to see this info in the >> traces, I have no objection adding it. >> >> Still waiting input from Steven Rostedt which I believe is the trace >> system maintainer concerning the hash-ptr situation. >> >> I did receive an auto-respond from him saying that he was in vacation >> until May 28th... > > Yep, I'm back now. > > Here's how it works using your patch as an example: > >> TP_fast_assign( >> __entry->ctx = ctx; >> + __entry->req = req; > > The "__entry" is a structure defined by TP_STRUCT__entry() that is located > on the ring buffer that can be read directly by user space (aka trace-cmd). > So yes, that value is never hashed, and one of the reasons that tracefs > requires root privilege to read it. > >> __entry->opcode = opcode; >> __entry->user_data = user_data; >> __entry->force_nonblock = force_nonblock; >> __entry->sq_thread = sq_thread; >> ), >> >> - TP_printk("ring %p, op %d, data 0x%llx, non block %d, sq_thread %d", >> - __entry->ctx, __entry->opcode, >> - (unsigned long long) __entry->user_data, >> - __entry->force_nonblock, __entry->sq_thread) >> + TP_printk("ring %p, req %p, op %d, data 0x%llx, non block %d, " >> + "sq_thread %d", __entry->ctx, __entry->req, >> + __entry->opcode, (unsigned long long)__entry->user_data, >> + __entry->force_nonblock, __entry->sq_thread) >> ); > > The TP_printk() macro *is* used when reading the "trace" or "trace_pipe" > file, and that uses vsnprintf() to process it. Which will hash the values > for %p (by default, because that's what it always did when vsnprintf() > started hashing values). > > Masami Hiramatsu added the hash-ptr option (which I told him to be the > default as that was the behavior before that option was created), where the > use could turn off the hashing. > > There's lots of trace events that expose the raw pointers when hash-ptr is > off or if the ring buffers are read via the trace_pip_raw interface. > > What's special about these pointers to hash them before they are recorded? io_uring offers all different operations and has internal request/memory recycling, so it may be an easy vector of attack in case of some vulnerabilities found, but nothing special. As that's the status quo, I wouldn't care, let's put aside my concerns and print them raw. -- Pavel Begunkov