linux-kernel - Re: [PATCH bpf-next v2 1/2] bpf: Add struct bpf_token

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAEf4BzbkfhqfpBt49h7SXYwbR1SK423pqf1328i8XujofjLYhQ@mail.gmail.com>
Date: Tue, 15 Jul 2025 14:52:33 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Tao Chen <chen.dylane@...ux.dev>
Cc: ast@...nel.org, daniel@...earbox.net, andrii@...nel.org, 
	martin.lau@...ux.dev, eddyz87@...il.com, song@...nel.org, 
	yonghong.song@...ux.dev, john.fastabend@...il.com, kpsingh@...nel.org, 
	sdf@...ichev.me, haoluo@...gle.com, jolsa@...nel.org, willemb@...gle.com, 
	kerneljasonxing@...il.com, bpf@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH bpf-next v2 1/2] bpf: Add struct bpf_token_info

On Mon, Jul 14, 2025 at 8:59 PM Tao Chen <chen.dylane@...ux.dev> wrote:
>
> The 'commit 35f96de04127 ("bpf: Introduce BPF token object")' added
> BPF token as a new kind of BPF kernel object. And BPF_OBJ_GET_INFO_BY_FD
> already used to get BPF object info, so we can also get token info with
> this cmd.
> One usage scenario, when program runs failed with token, because of
> the permission failure, we can report what BPF token is allowing with
> this API for debugging.
>
> Signed-off-by: Tao Chen <chen.dylane@...ux.dev>
> ---
>  include/linux/bpf.h            | 11 +++++++++++
>  include/uapi/linux/bpf.h       |  8 ++++++++
>  kernel/bpf/syscall.c           | 18 ++++++++++++++++++
>  kernel/bpf/token.c             | 28 +++++++++++++++++++++++++++-
>  tools/include/uapi/linux/bpf.h |  8 ++++++++
>  5 files changed, 72 insertions(+), 1 deletion(-)
>

LGTM, but see a nit below and in selftest patch

Acked-by: Andrii Nakryiko <andrii@...nel.org>

[...]

>
> +int bpf_token_get_info_by_fd(struct bpf_token *token,
> +                            const union bpf_attr *attr,
> +                            union bpf_attr __user *uattr)
> +{
> +       struct bpf_token_info __user *uinfo;
> +       struct bpf_token_info info;
> +       u32 info_copy, uinfo_len;
> +
> +       uinfo = u64_to_user_ptr(attr->info.info);
> +       uinfo_len = attr->info.info_len;
> +
> +       info_copy = min_t(u32, uinfo_len, sizeof(info));

you don't use info_len past this point, so just reassign it instead of
adding another variable (info_copy); seems like some other
get_info_by_fd functions use the same approach

> +       memset(&info, 0, sizeof(info));
> +
> +       info.allowed_cmds = token->allowed_cmds;
> +       info.allowed_maps = token->allowed_maps;
> +       info.allowed_progs = token->allowed_progs;
> +       info.allowed_attachs = token->allowed_attachs;
> +
> +       if (copy_to_user(uinfo, &info, info_copy) ||
> +           put_user(info_copy, &uattr->info.info_len))
> +               return -EFAULT;
> +
> +       return 0;
> +}
> +

[...]