* [PATCH bpf-next] bpf: Clean code with bpf_copy_to_user
@ 2025-07-03 12:43 Tao Chen
2025-07-03 15:35 ` Yonghong Song
0 siblings, 1 reply; 5+ messages in thread
From: Tao Chen @ 2025-07-03 12:43 UTC (permalink / raw)
To: ast, daniel, john.fastabend, andrii, martin.lau, eddyz87, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa
Cc: bpf, linux-kernel, Tao Chen
No logic change, just use bpf_copy_to_user to clean code.
Signed-off-by: Tao Chen <chen.dylane@linux.dev>
---
kernel/bpf/syscall.c | 17 +++--------------
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index e6eea594f1c..ca152d36312 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -5208,21 +5208,10 @@ static int bpf_task_fd_query_copy(const union bpf_attr *attr,
if (put_user(zero, ubuf))
return -EFAULT;
- } else if (input_len >= len + 1) {
- /* ubuf can hold the string with NULL terminator */
- if (copy_to_user(ubuf, buf, len + 1))
- return -EFAULT;
} else {
- /* ubuf cannot hold the string with NULL terminator,
- * do a partial copy with NULL terminator.
- */
- char zero = '\0';
-
- err = -ENOSPC;
- if (copy_to_user(ubuf, buf, input_len - 1))
- return -EFAULT;
- if (put_user(zero, ubuf + input_len - 1))
- return -EFAULT;
+ err = bpf_copy_to_user(ubuf, buf, input_len, len);
+ if (err)
+ return err;
}
}
--
2.48.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] bpf: Clean code with bpf_copy_to_user
2025-07-03 12:43 [PATCH bpf-next] bpf: Clean code with bpf_copy_to_user Tao Chen
@ 2025-07-03 15:35 ` Yonghong Song
2025-07-03 16:03 ` Tao Chen
0 siblings, 1 reply; 5+ messages in thread
From: Yonghong Song @ 2025-07-03 15:35 UTC (permalink / raw)
To: Tao Chen, ast, daniel, john.fastabend, andrii, martin.lau,
eddyz87, song, kpsingh, sdf, haoluo, jolsa
Cc: bpf, linux-kernel
On 7/3/25 5:43 AM, Tao Chen wrote:
> No logic change, just use bpf_copy_to_user to clean code.
>
> Signed-off-by: Tao Chen <chen.dylane@linux.dev>
> ---
> kernel/bpf/syscall.c | 17 +++--------------
> 1 file changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index e6eea594f1c..ca152d36312 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -5208,21 +5208,10 @@ static int bpf_task_fd_query_copy(const union bpf_attr *attr,
>
> if (put_user(zero, ubuf))
> return -EFAULT;
> - } else if (input_len >= len + 1) {
> - /* ubuf can hold the string with NULL terminator */
> - if (copy_to_user(ubuf, buf, len + 1))
> - return -EFAULT;
> } else {
> - /* ubuf cannot hold the string with NULL terminator,
> - * do a partial copy with NULL terminator.
> - */
> - char zero = '\0';
> -
> - err = -ENOSPC;
> - if (copy_to_user(ubuf, buf, input_len - 1))
> - return -EFAULT;
> - if (put_user(zero, ubuf + input_len - 1))
> - return -EFAULT;
> + err = bpf_copy_to_user(ubuf, buf, input_len, len);
> + if (err)
> + return err;
> }
> }
Actually, there is a return value change with this patch.
bpf_copy_to_user() return returns -ENOSPC while the original
implementation may return -EFAULT due to following code.
if (put_user(prog_id, &uattr->task_fd_query.prog_id) ||
put_user(fd_type, &uattr->task_fd_query.fd_type) ||
put_user(probe_offset, &uattr->task_fd_query.probe_offset) ||
put_user(probe_addr, &uattr->task_fd_query.probe_addr))
return -EFAULT;
return err;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] bpf: Clean code with bpf_copy_to_user
2025-07-03 15:35 ` Yonghong Song
@ 2025-07-03 16:03 ` Tao Chen
2025-07-03 16:14 ` Yonghong Song
0 siblings, 1 reply; 5+ messages in thread
From: Tao Chen @ 2025-07-03 16:03 UTC (permalink / raw)
To: Yonghong Song, ast, daniel, john.fastabend, andrii, martin.lau,
eddyz87, song, kpsingh, sdf, haoluo, jolsa
Cc: bpf, linux-kernel
在 2025/7/3 23:35, Yonghong Song 写道:
>
>
> On 7/3/25 5:43 AM, Tao Chen wrote:
>> No logic change, just use bpf_copy_to_user to clean code.
>>
>> Signed-off-by: Tao Chen <chen.dylane@linux.dev>
>> ---
>> kernel/bpf/syscall.c | 17 +++--------------
>> 1 file changed, 3 insertions(+), 14 deletions(-)
>>
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index e6eea594f1c..ca152d36312 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -5208,21 +5208,10 @@ static int bpf_task_fd_query_copy(const union
>> bpf_attr *attr,
>> if (put_user(zero, ubuf))
>> return -EFAULT;
>> - } else if (input_len >= len + 1) {
>> - /* ubuf can hold the string with NULL terminator */
>> - if (copy_to_user(ubuf, buf, len + 1))
>> - return -EFAULT;
>> } else {
>> - /* ubuf cannot hold the string with NULL terminator,
>> - * do a partial copy with NULL terminator.
>> - */
>> - char zero = '\0';
>> -
>> - err = -ENOSPC;
>> - if (copy_to_user(ubuf, buf, input_len - 1))
>> - return -EFAULT;
>> - if (put_user(zero, ubuf + input_len - 1))
>> - return -EFAULT;
>> + err = bpf_copy_to_user(ubuf, buf, input_len, len);
>> + if (err)
>> + return err;
>> }
>> }
>
> Actually, there is a return value change with this patch.
> bpf_copy_to_user() return returns -ENOSPC while the original
> implementation may return -EFAULT due to following code.
>
> if (put_user(prog_id, &uattr->task_fd_query.prog_id) ||
> put_user(fd_type, &uattr->task_fd_query.fd_type) ||
> put_user(probe_offset, &uattr->task_fd_query.probe_offset) ||
> put_user(probe_addr, &uattr->task_fd_query.probe_addr))
> return -EFAULT;
>
> return err;
>
You are right, maybe we can just use:
err = bpf_copy_to_user(ubuf, buf, input_len, len);
and no return check
or move these put_user code to the front.
--
Best Regards
Tao Chen
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] bpf: Clean code with bpf_copy_to_user
2025-07-03 16:03 ` Tao Chen
@ 2025-07-03 16:14 ` Yonghong Song
2025-07-03 16:19 ` Tao Chen
0 siblings, 1 reply; 5+ messages in thread
From: Yonghong Song @ 2025-07-03 16:14 UTC (permalink / raw)
To: Tao Chen, ast, daniel, john.fastabend, andrii, martin.lau,
eddyz87, song, kpsingh, sdf, haoluo, jolsa
Cc: bpf, linux-kernel
On 7/3/25 9:03 AM, Tao Chen wrote:
> 在 2025/7/3 23:35, Yonghong Song 写道:
>>
>>
>> On 7/3/25 5:43 AM, Tao Chen wrote:
>>> No logic change, just use bpf_copy_to_user to clean code.
>>>
>>> Signed-off-by: Tao Chen <chen.dylane@linux.dev>
>>> ---
>>> kernel/bpf/syscall.c | 17 +++--------------
>>> 1 file changed, 3 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>>> index e6eea594f1c..ca152d36312 100644
>>> --- a/kernel/bpf/syscall.c
>>> +++ b/kernel/bpf/syscall.c
>>> @@ -5208,21 +5208,10 @@ static int bpf_task_fd_query_copy(const
>>> union bpf_attr *attr,
>>> if (put_user(zero, ubuf))
>>> return -EFAULT;
>>> - } else if (input_len >= len + 1) {
>>> - /* ubuf can hold the string with NULL terminator */
>>> - if (copy_to_user(ubuf, buf, len + 1))
>>> - return -EFAULT;
>>> } else {
>>> - /* ubuf cannot hold the string with NULL terminator,
>>> - * do a partial copy with NULL terminator.
>>> - */
>>> - char zero = '\0';
>>> -
>>> - err = -ENOSPC;
>>> - if (copy_to_user(ubuf, buf, input_len - 1))
>>> - return -EFAULT;
>>> - if (put_user(zero, ubuf + input_len - 1))
>>> - return -EFAULT;
>>> + err = bpf_copy_to_user(ubuf, buf, input_len, len);
>>> + if (err)
>>> + return err;
>>> }
>>> }
>>
>> Actually, there is a return value change with this patch.
>> bpf_copy_to_user() return returns -ENOSPC while the original
>> implementation may return -EFAULT due to following code.
>>
>> if (put_user(prog_id, &uattr->task_fd_query.prog_id) ||
>> put_user(fd_type, &uattr->task_fd_query.fd_type) ||
>> put_user(probe_offset,
>> &uattr->task_fd_query.probe_offset) ||
>> put_user(probe_addr, &uattr->task_fd_query.probe_addr))
>> return -EFAULT;
>>
>> return err;
>>
>
> You are right, maybe we can just use:
> err = bpf_copy_to_user(ubuf, buf, input_len, len);
> and no return check
> or move these put_user code to the front.
Maybe do the following?
err = bpf_copy_to_user(ubuf, buf, input_len, len);
if (err && err != -ENOSPC)
return err;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] bpf: Clean code with bpf_copy_to_user
2025-07-03 16:14 ` Yonghong Song
@ 2025-07-03 16:19 ` Tao Chen
0 siblings, 0 replies; 5+ messages in thread
From: Tao Chen @ 2025-07-03 16:19 UTC (permalink / raw)
To: Yonghong Song, ast, daniel, john.fastabend, andrii, martin.lau,
eddyz87, song, kpsingh, sdf, haoluo, jolsa
Cc: bpf, linux-kernel
在 2025/7/4 00:14, Yonghong Song 写道:
>
>
> On 7/3/25 9:03 AM, Tao Chen wrote:
>> 在 2025/7/3 23:35, Yonghong Song 写道:
>>>
>>>
>>> On 7/3/25 5:43 AM, Tao Chen wrote:
>>>> No logic change, just use bpf_copy_to_user to clean code.
>>>>
>>>> Signed-off-by: Tao Chen <chen.dylane@linux.dev>
>>>> ---
>>>> kernel/bpf/syscall.c | 17 +++--------------
>>>> 1 file changed, 3 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>>>> index e6eea594f1c..ca152d36312 100644
>>>> --- a/kernel/bpf/syscall.c
>>>> +++ b/kernel/bpf/syscall.c
>>>> @@ -5208,21 +5208,10 @@ static int bpf_task_fd_query_copy(const
>>>> union bpf_attr *attr,
>>>> if (put_user(zero, ubuf))
>>>> return -EFAULT;
>>>> - } else if (input_len >= len + 1) {
>>>> - /* ubuf can hold the string with NULL terminator */
>>>> - if (copy_to_user(ubuf, buf, len + 1))
>>>> - return -EFAULT;
>>>> } else {
>>>> - /* ubuf cannot hold the string with NULL terminator,
>>>> - * do a partial copy with NULL terminator.
>>>> - */
>>>> - char zero = '\0';
>>>> -
>>>> - err = -ENOSPC;
>>>> - if (copy_to_user(ubuf, buf, input_len - 1))
>>>> - return -EFAULT;
>>>> - if (put_user(zero, ubuf + input_len - 1))
>>>> - return -EFAULT;
>>>> + err = bpf_copy_to_user(ubuf, buf, input_len, len);
>>>> + if (err)
>>>> + return err;
>>>> }
>>>> }
>>>
>>> Actually, there is a return value change with this patch.
>>> bpf_copy_to_user() return returns -ENOSPC while the original
>>> implementation may return -EFAULT due to following code.
>>>
>>> if (put_user(prog_id, &uattr->task_fd_query.prog_id) ||
>>> put_user(fd_type, &uattr->task_fd_query.fd_type) ||
>>> put_user(probe_offset, &uattr-
>>> >task_fd_query.probe_offset) ||
>>> put_user(probe_addr, &uattr->task_fd_query.probe_addr))
>>> return -EFAULT;
>>>
>>> return err;
>>>
>>
>> You are right, maybe we can just use:
>> err = bpf_copy_to_user(ubuf, buf, input_len, len);
>> and no return check
>> or move these put_user code to the front.
>
> Maybe do the following?
>
> err = bpf_copy_to_user(ubuf, buf, input_len, len);
> if (err && err != -ENOSPC)
> return err;
>
Well, it looks better, i will change it in v2, thanks.
--
Best Regards
Tao Chen
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-07-03 16:20 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-03 12:43 [PATCH bpf-next] bpf: Clean code with bpf_copy_to_user Tao Chen
2025-07-03 15:35 ` Yonghong Song
2025-07-03 16:03 ` Tao Chen
2025-07-03 16:14 ` Yonghong Song
2025-07-03 16:19 ` Tao Chen
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.