On 01/15/2018 07:38 AM, Y Song wrote:
> On Fri, Jan 12, 2018 at 11:23 AM, Daniel Borkmann <dan...@iogearbox.net>
> wrote:
[...]
>>
>> I've been thinking to additionally reject arithmetic on ctx
>> pointer in adjust_ptr_min_max_vals() right upfront as well
>> since we reject actual access in such case later on anyway,
>> but there's a use case in tracing (in bcc) in combination
>> with passing such ctx to bpf_probe_read(), so we cannot do
>> that part.
>
> There is a reason why bcc does this. For example, suppose that we want to
> trace kernel tracepoint, sched_process_exec,
>
> TRACE_EVENT(sched_process_exec,
>
> TP_PROTO(struct task_struct *p, pid_t old_pid,
> struct linux_binprm *bprm),
>
> TP_ARGS(p, old_pid, bprm),
>
> TP_STRUCT__entry(
> __string( filename, bprm->filename )
> __field( pid_t, pid )
> __field( pid_t, old_pid )
> ),
>
> TP_fast_assign(
> __assign_str(filename, bprm->filename);
> __entry->pid = p->pid;
> __entry->old_pid = old_pid;
> ),
>
> TP_printk("filename=%s pid=%d old_pid=%d", __get_str(filename),
> __entry->pid, __entry->old_pid)
> );
>
> Eventually structure at
> /sys/kernel/debug/tracing/event/sched/sched_process_exec/format:
> ......
> field:__data_loc char[] filename; offset:8;
> size:4; signed:1;
> field:pid_t pid; offset:12; size:4; signed:1;
> field:pid_t old_pid; offset:16; size:4; signed:1;
>
> and "data_loc filename" is the offset in the structure where
> "filename" is stored.
>
> Therefore, in bcc, the access sequence is:
> offset = args->filename; /* field __data_loc filename */
> bpf_probe_read(&dst, len, (char *)args + offset);
>
> For this kind of dynamic array in the tracepoint, the offset to access
> certain field in ctx will be unknown at verification time.
Right, that is exactly what I said in above paragraph.
> So I suggest to remove the above paragraph regarding to potential ctx+offset
> rejection.
I'm confused, I mentioned we cannot reject exactly because of this
use-case, I thought it's worth having it in the log for future
reference so we don't forget about it since it's not too obvious.
Cheers,
Daniel
