All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tao Chen <chen.dylane@linux.dev>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>,
	Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Martin KaFai Lau <martin.lau@linux.dev>,
	Eduard <eddyz87@gmail.com>, Song Liu <song@kernel.org>,
	Yonghong Song <yonghong.song@linux.dev>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@kernel.org>,
	Stanislav Fomichev <sdf@fomichev.me>, Hao Luo <haoluo@google.com>,
	Jiri Olsa <jolsa@kernel.org>, bpf <bpf@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH bpf-next 1/2] bpf: Add bpf_get_task_cmdline kfunc
Date: Wed, 26 Nov 2025 17:15:12 +0800	[thread overview]
Message-ID: <ff4be1ef-ef02-471e-b8f0-8e9cdb312794@linux.dev> (raw)
In-Reply-To: <CAEf4BzaXozVSTsC7XZ8Ojkju1szk65nAg8Zc5Y_2OVewKV4heA@mail.gmail.com>

在 2025/11/26 07:32, Andrii Nakryiko 写道:
> On Fri, Nov 21, 2025 at 5:17 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
>>
>> On Tue, Nov 18, 2025 at 4:58 AM Tao Chen <chen.dylane@linux.dev> wrote:
>>>
>>> Add the bpf_get_task_cmdline kfunc. One use case is as follows: In
>>> production environments, there are often short-lived script tasks executed,
>>> and sometimes these tasks may cause stability issues. It is desirable to
>>> detect these script tasks via eBPF. The common approach is to check
>>> the process name, but it can be difficult to distinguish specific
>>> tasks in some cases. Take the shell as an example: some tasks are
>>> started via bash xxx.sh – their process name is bash, but the script
>>> name of the task can be obtained through the cmdline. Additionally,
>>> myabe this is helpful for security auditing purposes.
>>
>> maybe
>>
>>>
>>> Signed-off-by: Tao Chen <chen.dylane@linux.dev>
>>> ---
>>>   kernel/bpf/helpers.c | 22 ++++++++++++++++++++++
>>>   1 file changed, 22 insertions(+)
>>>
>>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
>>> index 865b0dae38d..7cac17d58d5 100644
>>> --- a/kernel/bpf/helpers.c
>>> +++ b/kernel/bpf/helpers.c
>>> @@ -2685,6 +2685,27 @@ __bpf_kfunc struct task_struct *bpf_task_from_pid(s32 pid)
>>>          return p;
>>>   }
>>>
>>> +/*
>>> + * bpf_get_task_cmdline - Get the cmdline to a buffer
>>> + *
>>> + * @task: The task whose cmdline to get.
>>> + * @buffer: The buffer to save cmdline info.
>>> + * @len: The length of the buffer.
>>> + *
>>> + * Return: the size of the cmdline field copied. Note that the copy does
>>> + * not guarantee an ending NULL byte. A negative error code on failure.
>>> + */
>>> +__bpf_kfunc int bpf_get_task_cmdline(struct task_struct *task, char *buffer, size_t len)
>>
>> 'size_t len' doesn't make the verifier track the size of the buffer.
>> while 'char *buffer' tells the verifier to check that _one_ byte is available.
>> So this is buggy.
>>
>> In general the kfunc seems useful, but selftest in patch 2 is just bad
>>
> 
> Besides that mm->arg_lock spinlock (which I don't think matters all
> that much for BPF programs), is there anything special in
> get_cmdline() that BPF program cannot just implemented? Ultimately,
> it's just copying mm->arg_start and mm->env_start zero-separated
> strings, no? We have bpf_copy_from_user_task_str() and also
> dynptr-based equivalent of it for even more variable-length
> flexibility. That should be all one needs, no?
> 

 From a quick look at how both are implemented, it seems that way.
Hold off on this patch for now. I will move forward if we find something 
new.

>> + ret = bpf_get_task_cmdline(task, buf, sizeof(buf));
>> + if (ret < 0)
>> +    err = 1;
>> +
>> + return 0;
>> +}
>>
>> it's not testing much.
>>
>> Also you must explain the true motivation for the kfunc.
>> "maybe helpful for security" is too vague.
>> Do you have a proprietary bpf-lsm that needs it?
>> What is the exact use case?
>>
>> pw-bot: cr


-- 
Best Regards
Tao Chen

      reply	other threads:[~2025-11-26  9:15 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-18 12:58 [PATCH bpf-next 1/2] bpf: Add bpf_get_task_cmdline kfunc Tao Chen
2025-11-18 12:58 ` [PATCH bpf-next 2/2] selftests/bpf: Add bpf_get_task_cmdline test case Tao Chen
2025-11-22  1:17 ` [PATCH bpf-next 1/2] bpf: Add bpf_get_task_cmdline kfunc Alexei Starovoitov
2025-11-25 23:32   ` Andrii Nakryiko
2025-11-26  9:15     ` Tao Chen [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ff4be1ef-ef02-471e-b8f0-8e9cdb312794@linux.dev \
    --to=chen.dylane@linux.dev \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=sdf@fomichev.me \
    --cc=song@kernel.org \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.