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

[Yonghong Song <yonghong.song@linux.dev>]

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;