* [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.