[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <31dac2d6-da40-4b7c-a8ca-d67891e7326d@linux.dev>
Date: Wed, 16 Jul 2025 13:29:29 +0800
From: Tao Chen <chen.dylane@...ux.dev>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
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
在 2025/7/16 05:52, Andrii Nakryiko 写道:
> 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
>
will change it in v3, thanks.
>> + 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;
>> +}
>> +
>
> [...]
--
Best Regards
Tao Chen
Powered by blists - more mailing lists