On Tue, Jan 07, 2020 at 06:28:11PM +0000, Yonghong Song wrote:
SNIP
> >> index ed2075884724..650df4ed346e 100644
> >> --- a/kernel/bpf/btf.c
> >> +++ b/kernel/bpf/btf.c
> >> @@ -3633,7 +3633,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
> >> const struct bpf_prog *prog,
> >> struct bpf_insn_access_aux *info)
> >> {
> >> - const struct btf_type *t = prog->aux->attach_func_proto;
> >> + const struct btf_type *tp, *t = prog->aux->attach_func_proto;
> >> struct bpf_prog *tgt_prog = prog->aux->linked_prog;
> >> struct btf *btf = bpf_prog_get_target_btf(prog);
> >> const char *tname = prog->aux->attach_func_name;
> >> @@ -3695,6 +3695,17 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
> >> */
> >> return true;
> >>
> >> + tp = btf_type_by_id(btf, t->type);
> >> + /* skip modifiers */
> >> + while (btf_type_is_modifier(tp))
> >> + tp = btf_type_by_id(btf, tp->type);
> >> +
> >> + if (btf_type_is_int(tp))
> >> + /* This is a pointer scalar.
> >> + * It is the same as scalar from the verifier safety pov.
> >> + */
> >> + return true;
> >
> > This should work since:
> > - the int pointer will be treated as a scalar later on
> > - bpf_probe_read() will be used to read the contents
> >
> > I am wondering whether we should add proper verifier support
> > to allow pointer to int ctx access. There, users do not need
> > to use bpf_probe_read() to dereference the pointer.
> >
> > Discussed with Martin, maybe somewhere in check_ptr_to_btf_access(),
> > before btf_struct_access(), checking if it is a pointer to int/enum,
> > it should just allow and return SCALAR_VALUE.
>
> double checked check_ptr_to_btf_access() and btf_struct_access().
> btf_struct_access() already returns SCALAR_VALUE for pointer to
> int/enum. So verifier change is probably not needed.
ok, great
>
> In your above code, could you do
> btf_type_is_int(t) || btf_type_is_enum(t)
> which will cover pointer to enum as well?
sure, I'll include that
thanks,
jirka
