Re: [PATCH bpf-next v2 1/4] bpf: BPF token support for BPF_BTF_GET_FD_BY_ID

[Jiri Olsa <olsajiri@gmail.com>]

On Wed, Mar 05, 2025 at 07:49:39PM +0000, Mykyta Yatsenko wrote:
> From: Mykyta Yatsenko <yatsenko@meta.com>
> 
> Currently BPF_BTF_GET_FD_BY_ID requires CAP_SYS_ADMIN, which does not
> allow running it from user namespace. This creates a problem when
> freplace program running from user namespace needs to query target
> program BTF.
> This patch relaxes capable check from CAP_SYS_ADMIN to CAP_BPF and adds
> support for BPF token that can be passed in attributes to syscall.
> 
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> ---
>  include/uapi/linux/bpf.h                                 | 1 +
>  kernel/bpf/syscall.c                                     | 9 +++++++--
>  tools/include/uapi/linux/bpf.h                           | 1 +
>  .../selftests/bpf/prog_tests/libbpf_get_fd_by_id_opts.c  | 3 +--
>  4 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index bb37897c0393..73c23daacabf 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1652,6 +1652,7 @@ union bpf_attr {
>  		};
>  		__u32		next_id;
>  		__u32		open_flags;
> +		__s32		token_fd;
>  	};
>  
>  	struct { /* anonymous struct used by BPF_OBJ_GET_INFO_BY_FD */
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 57a438706215..6975d391bb05 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -5137,14 +5137,19 @@ static int bpf_btf_load(const union bpf_attr *attr, bpfptr_t uattr, __u32 uattr_
>  	return btf_new_fd(attr, uattr, uattr_size);
>  }
>  
> -#define BPF_BTF_GET_FD_BY_ID_LAST_FIELD btf_id
> +#define BPF_BTF_GET_FD_BY_ID_LAST_FIELD token_fd
>  
>  static int bpf_btf_get_fd_by_id(const union bpf_attr *attr)
>  {
> +	struct bpf_token *token = NULL;
> +
>  	if (CHECK_ATTR(BPF_BTF_GET_FD_BY_ID))
>  		return -EINVAL;
>  
> -	if (!capable(CAP_SYS_ADMIN))
> +	if (attr->open_flags & BPF_F_TOKEN_FD)
> +		token = bpf_token_get_from_fd(attr->token_fd);

hi,
I think you need to check token in here with IS_ERR(token)
and call bpf_token_allow_cmd

> +
> +	if (!bpf_token_capable(token, CAP_SYS_ADMIN))

and bpf_token_put in here

jirka

>  		return -EPERM;
>  
>  	return btf_get_fd_by_id(attr->btf_id);
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index bb37897c0393..73c23daacabf 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -1652,6 +1652,7 @@ union bpf_attr {
>  		};
>  		__u32		next_id;
>  		__u32		open_flags;
> +		__s32		token_fd;
>  	};
>  
>  	struct { /* anonymous struct used by BPF_OBJ_GET_INFO_BY_FD */
> diff --git a/tools/testing/selftests/bpf/prog_tests/libbpf_get_fd_by_id_opts.c b/tools/testing/selftests/bpf/prog_tests/libbpf_get_fd_by_id_opts.c
> index a3f238f51d05..976ff38a6d43 100644
> --- a/tools/testing/selftests/bpf/prog_tests/libbpf_get_fd_by_id_opts.c
> +++ b/tools/testing/selftests/bpf/prog_tests/libbpf_get_fd_by_id_opts.c
> @@ -75,9 +75,8 @@ void test_libbpf_get_fd_by_id_opts(void)
>  	if (!ASSERT_EQ(ret, -EINVAL, "bpf_link_get_fd_by_id_opts"))
>  		goto close_prog;
>  
> -	/* BTF get fd with opts set should not work (no kernel support). */
>  	ret = bpf_btf_get_fd_by_id_opts(0, &fd_opts_rdonly);
> -	ASSERT_EQ(ret, -EINVAL, "bpf_btf_get_fd_by_id_opts");
> +	ASSERT_EQ(ret, -ENOENT, "bpf_btf_get_fd_by_id_opts");
>  
>  close_prog:
>  	if (fd >= 0)
> -- 
> 2.48.1
> 
>