1. Patchew
  2. linux
  3. View series

[PATCH bpf-next] bpf: Clean code with bpf_copy_to_user

Tao Chen posted 1 patch 3 months ago
There is a newer version of this series
kernel/bpf/syscall.c | 17 +++--------------
1 file changed, 3 insertions(+), 14 deletions(-)
[PATCH bpf-next] bpf: Clean code with bpf_copy_to_user
Posted by Tao Chen 3 months ago 
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
Re: [PATCH bpf-next] bpf: Clean code with bpf_copy_to_user
Posted by Yonghong Song 3 months ago 

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;
Re: [PATCH bpf-next] bpf: Clean code with bpf_copy_to_user
Posted by Tao Chen 3 months ago 
在 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
Re: [PATCH bpf-next] bpf: Clean code with bpf_copy_to_user
Posted by Yonghong Song 3 months ago 

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;
Re: [PATCH bpf-next] bpf: Clean code with bpf_copy_to_user
Posted by Tao Chen 3 months ago 
在 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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.