[bpf-next,v2,0/3] Annotate kfuncs in .BTF_ids section

=== Description ===

This is a bpf-treewide change that annotates all kfuncs as such inside
.BTF_ids. This annotation eventually allows us to automatically generate
kfunc prototypes from bpftool.

We store this metadata inside a yet-unused flags field inside struct
btf_id_set8 (thanks Kumar!). pahole will be taught where to look.

More details about the full chain of events are available in commit 3's
description.

The accompanying pahole changes (still needs some cleanup) can be viewed
here on this "frozen" branch [0].

[0]: https://github.com/danobi/pahole/tree/kfunc_btf-mailed

=== Changelog ===

Changes from v1:
* Move WARN_ON() up a call level
* Also return error when kfunc set is not properly tagged
* Use BTF_KFUNCS_START/END instead of flags
* Rename BTF_SET8_KFUNC to BTF_SET8_KFUNCS

Daniel Xu (3):
  bpf: btf: Support flags for BTF_SET8 sets
  bpf: btf: Add BTF_KFUNCS_START/END macro pair
  bpf: treewide: Annotate BPF kfuncs in BTF

 drivers/hid/bpf/hid_bpf_dispatch.c            |  8 +++----
 fs/verity/measure.c                           |  4 ++--
 include/linux/btf_ids.h                       | 21 +++++++++++++++----
 kernel/bpf/btf.c                              |  4 ++++
 kernel/bpf/cpumask.c                          |  4 ++--
 kernel/bpf/helpers.c                          |  8 +++----
 kernel/bpf/map_iter.c                         |  4 ++--
 kernel/cgroup/rstat.c                         |  4 ++--
 kernel/trace/bpf_trace.c                      |  8 +++----
 net/bpf/test_run.c                            |  8 +++----
 net/core/filter.c                             | 16 +++++++-------
 net/core/xdp.c                                |  4 ++--
 net/ipv4/bpf_tcp_ca.c                         |  4 ++--
 net/ipv4/fou_bpf.c                            |  4 ++--
 net/ipv4/tcp_bbr.c                            |  4 ++--
 net/ipv4/tcp_cubic.c                          |  4 ++--
 net/ipv4/tcp_dctcp.c                          |  4 ++--
 net/netfilter/nf_conntrack_bpf.c              |  4 ++--
 net/netfilter/nf_nat_bpf.c                    |  4 ++--
 net/xfrm/xfrm_interface_bpf.c                 |  4 ++--
 net/xfrm/xfrm_state_bpf.c                     |  4 ++--
 .../selftests/bpf/bpf_testmod/bpf_testmod.c   |  8 +++----
 22 files changed, 77 insertions(+), 60 deletions(-)

On Thu, Jan 04, 2024 at 07:45:49PM -0700, Daniel Xu wrote:

SNIP

> diff --git a/fs/verity/measure.c b/fs/verity/measure.c
> index bf7a5f4cccaf..3969d54158d1 100644
> --- a/fs/verity/measure.c
> +++ b/fs/verity/measure.c
> @@ -159,9 +159,9 @@ __bpf_kfunc int bpf_get_fsverity_digest(struct file *file, struct bpf_dynptr_ker
>  
>  __bpf_kfunc_end_defs();
>  
> -BTF_SET8_START(fsverity_set_ids)
> +BTF_KFUNCS_START(fsverity_set_ids)
>  BTF_ID_FLAGS(func, bpf_get_fsverity_digest, KF_TRUSTED_ARGS)
> -BTF_SET8_END(fsverity_set_ids)
> +BTF_KFUNCS_END(fsverity_set_ids)
>  
>  static int bpf_get_fsverity_digest_filter(const struct bpf_prog *prog, u32 kfunc_id)
>  {
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 51e8b4bee0c8..8cc718f37a9d 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -7802,6 +7802,10 @@ int register_btf_kfunc_id_set(enum bpf_prog_type prog_type,
>  {
>  	enum btf_kfunc_hook hook;
>  
> +	/* All kfuncs need to be tagged as such in BTF */
> +	if (WARN_ON(!(kset->set->flags & BTF_SET8_KFUNCS)))
> +		return -EINVAL;

having the warning for module with wrong set8 flags seems wrong to me,
I think we should trigger the warn only for kernel calls.. by adding
kset->owner check in the condition above

jirka

> +
>  	hook = bpf_prog_type_to_kfunc_hook(prog_type);
>  	return __register_btf_kfunc_id_set(hook, kset);
>  }

SNIP

On Fri, Jan 05, 2024 at 09:55:43AM -0700, Daniel Xu wrote:
> On Fri, Jan 05, 2024 at 04:11:33PM +0100, Jiri Olsa wrote:
> > On Thu, Jan 04, 2024 at 07:45:49PM -0700, Daniel Xu wrote:
> > 
> > SNIP
> > 
> > > diff --git a/fs/verity/measure.c b/fs/verity/measure.c
> > > index bf7a5f4cccaf..3969d54158d1 100644
> > > --- a/fs/verity/measure.c
> > > +++ b/fs/verity/measure.c
> > > @@ -159,9 +159,9 @@ __bpf_kfunc int bpf_get_fsverity_digest(struct file *file, struct bpf_dynptr_ker
> > >  
> > >  __bpf_kfunc_end_defs();
> > >  
> > > -BTF_SET8_START(fsverity_set_ids)
> > > +BTF_KFUNCS_START(fsverity_set_ids)
> > >  BTF_ID_FLAGS(func, bpf_get_fsverity_digest, KF_TRUSTED_ARGS)
> > > -BTF_SET8_END(fsverity_set_ids)
> > > +BTF_KFUNCS_END(fsverity_set_ids)
> > >  
> > >  static int bpf_get_fsverity_digest_filter(const struct bpf_prog *prog, u32 kfunc_id)
> > >  {
> > > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> > > index 51e8b4bee0c8..8cc718f37a9d 100644
> > > --- a/kernel/bpf/btf.c
> > > +++ b/kernel/bpf/btf.c
> > > @@ -7802,6 +7802,10 @@ int register_btf_kfunc_id_set(enum bpf_prog_type prog_type,
> > >  {
> > >  	enum btf_kfunc_hook hook;
> > >  
> > > +	/* All kfuncs need to be tagged as such in BTF */
> > > +	if (WARN_ON(!(kset->set->flags & BTF_SET8_KFUNCS)))
> > > +		return -EINVAL;
> > 
> > having the warning for module with wrong set8 flags seems wrong to me,
> > I think we should trigger the warn only for kernel calls.. by adding
> > kset->owner check in the condition above
> 
> Just checking:
> 
> The reasoning is that =m and out-of-tree modules can and should check
> return code, right?
> 
> And =y modules or vmlinux-based registrations do not check return code,
> so WARN() is necessary?
> 
> If so, I'd agree.

right, I was also concerned we could flood console with loading module
that just uses wrong set8.. perhaps we could just use WARN_ON_ONCE with
no additional checks

jirka