On Tue, Mar 11, 2025 at 2:54 PM Mykyta Yatsenko
<mykyta.yatsenko5@xxxxxxxxx> wrote:
>
> From: Mykyta Yatsenko <yatsenko@xxxxxxxx>
>
> 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@xxxxxxxx>
> ---
> include/uapi/linux/bpf.h | 1 +
> kernel/bpf/syscall.c | 20 +++++++++++++++++--
> tools/include/uapi/linux/bpf.h | 1 +
> .../bpf/prog_tests/libbpf_get_fd_by_id_opts.c | 3 +--
> 4 files changed, 21 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..188f7296cf9f 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -5137,15 +5137,31 @@ 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;
as I mentioned in another email, we used to implicitly reject any
open_flags set because of BPF_BTF_GET_FD_BY_ID_LAST_FIELD btf_id, but
now that we do support some flags in open_flags, we need to validate
that there are no other flags. So you need something like:
if (attr->open_flags & ~BPF_F_TOKEN_FD)
return -EINVAL;
pw-bot: cr
>
> - if (!capable(CAP_SYS_ADMIN))
> + if (attr->open_flags & BPF_F_TOKEN_FD) {
> + token = bpf_token_get_from_fd(attr->token_fd);
> + if (IS_ERR(token))
> + return PTR_ERR(token);
> + if (!bpf_token_allow_cmd(token, BPF_BTF_GET_FD_BY_ID)) {
> + bpf_token_put(token);
> + token = NULL;
> + }
> + }
> +
> + if (!bpf_token_capable(token, CAP_SYS_ADMIN)) {
> + bpf_token_put(token);
> return -EPERM;
> + }
> +
> + bpf_token_put(token);
>
> 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
>
