linux-kernel - Re: [PATCH bpf-next] bpf: use type_may_be_null() helper for nullable-param check

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

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

Message-ID: <u33xtqql46ppe2ebqj7u26so4b7my6ebsdeoxdb6kn57ygbniq@3vmbbct3hphm>
Date: Fri, 6 Sep 2024 10:10:40 +0800
From: Shung-Hsi Yu <shung-hsi.yu@...e.com>
To: Matt Bobrowski <mattbobrowski@...gle.com>
Cc: Eduard Zingerman <eddyz87@...il.com>, bpf@...r.kernel.org, 
	Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, 
	Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, 
	Song Liu <song@...nel.org>, Yonghong Song <yonghong.song@...ux.dev>, 
	John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>, 
	Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	David Vernet <void@...ifault.com>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org, 
	kernel test robot <lkp@...el.com>
Subject: Re: [PATCH bpf-next] bpf: use type_may_be_null() helper for
 nullable-param check

On Thu, Sep 05, 2024 at 08:00:09AM GMT, Matt Bobrowski wrote:
> On Thu, Sep 05, 2024 at 01:52:32PM +0800, Shung-Hsi Yu wrote:
[...]
> > --- a/net/bpf/bpf_dummy_struct_ops.c
> > +++ b/net/bpf/bpf_dummy_struct_ops.c
> > @@ -115,7 +115,7 @@ static int check_test_run_args(struct bpf_prog *prog, struct bpf_dummy_ops_test_
> >  
> >  		offset = btf_ctx_arg_offset(bpf_dummy_ops_btf, func_proto, arg_no);
> >  		info = find_ctx_arg_info(prog->aux, offset);
> > -		if (info && (info->reg_type & PTR_MAYBE_NULL))
> > +		if (info && type_may_be_null(info->reg_type))
> 
> Maybe as part of this clean up, we should also consider replacing all
> the open-coded & PTR_MAYBE_NULL checks with type_may_be_null() which
> we have sprinkled throughout kernel/bpf/verifier.c?

Agree we should. Usage like this could be replaced

	if (ptr_reg->type & PTR_MAYBE_NULL) {
		verbose(env, "R%d pointer arithmetic on %s prohibited, null-check it first\n",
			dst, reg_type_str(env, ptr_reg->type));
		return -EACCES;
	}

OTOH replacing & PTR_MAYBE_NULL here probably won't help improve
clarity.

	if (base_type(arg->arg_type) == ARG_PTR_TO_BTF_ID) {
		reg->type = PTR_TO_BTF_ID;
		if (arg->arg_type & PTR_MAYBE_NULL)
			reg->type |= PTR_MAYBE_NULL;
		if (arg->arg_type & PTR_UNTRUSTED)
			reg->type |= PTR_UNTRUSTED;
		if (arg->arg_type & PTR_TRUSTED)
			reg->type |= PTR_TRUSTED;
		...

For such case we might need to introduce another helper (bitwise-OR
between enum bpf_type_flag should be free of compiler warning).

	reg->type = type_flag_apply(PTR_TO_BTF_ID, arg->arg_type,
								PTR_MAYBE_NULL | PTR_UNTRUSTED | PTR_TRUSTED);

WDYT?