Re: [PATCH] tracing: eprobe: read the complete FILTER_PTR_STRING pointer

[Martin Kaiser]

Thus wrote Masami Hiramatsu (mhiramat@xxxxxxxxxx):

> Ah, OK. I understand the problem.

>  - ring buffer and its records should be self-contained.
>  - In most cases, events use __data_loc/__rel_loc or fixed array to store
>    strings.
>  - only syscall events exposes the char *, which is not recommended but
>    important to debug user space. (not for dereference)

> The example usage of FILTER_PTR_STRING is actually using FILTER_STATIC_STRING
> now, so FILTER_PTR_STRING is left broken. (hmm, but there are many
>  "const char *" are used especially under rcu events...)

> OK, can you update your patch description to use rcu events?

I've just sent a v2 with an rcu event as an example.

> BTW, I think those also should be decoded from enum value in the events,
> or use __rel_loc. Since it is not self-contained. (it's a TODO item)

That makes sense. But it needs a bit more space in the ringbuffer for each
event.

> > > I think better solution is fixing sycall tracer.

> > I would say that syscall trace is doing the right thing. The ringbuffer entry
> > is a struct syscall_trace_enter, the syscall arguments are unsigned longs.
> > They are written in ftrace_syscall_enter, this looks correct to me.

> OK, I thought the filename points the ringbuffer, but it actually points
> the user space. (saving a raw parameter values) So it is OK.

> For eprobe users, it should not access to the user space data directly
> because it can cause page fault in the kernel without fixup. It may work
> on x86, but it doesn't work on other architecture which has separated
> address space for user space. To avoid such mistake, it saves actual
> string in the ringbuffer as __filename_val.

> Hmm, this must be documented in eprobe example code...

Could we use is_kernel() from kallsyms.h to check the address?

Or should we forbid string and ustring fetch types in eprobes if the
base field is a FILTER_PTR_STRING?

Best regards,
Martin

> > A const char * syscall argument is using FILTER_PTR_STRING, the unsigned long
> > argument from the ringbuffer is read as a char and then converted to a
> > truncated pointer.


> Thanks,

> -- 
> Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>