netdev - Re: Re: [PATCH 1/5] bpf: Allow non struct type for btf ctx access

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 8 Jan 2020 15:38:59 +0100
From:   Jiri Olsa <jolsa@...hat.com>
To:     Yonghong Song <yhs@...com>
Cc:     Jiri Olsa <jolsa@...nel.org>, Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        Andrii Nakryiko <andriin@...com>, Martin Lau <kafai@...com>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>,
        David Miller <davem@...hat.com>
Subject: Re: Re: [PATCH 1/5] bpf: Allow non struct type for btf ctx access

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