* [bpf-next] bpf: use kvmemdup_bpfptr helper
@ 2022-09-09 10:58 Wang Yufen
2022-09-09 17:27 ` sdf
0 siblings, 1 reply; 3+ messages in thread
From: Wang Yufen @ 2022-09-09 10:58 UTC (permalink / raw)
To: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa
Cc: bpf, linux-kernel
Use kvmemdup_bpfptr helper instead of open-coding to
simplify the code.
Signed-off-by: Wang Yufen <wangyufen@huawei.com>
---
kernel/bpf/syscall.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 4e9d4622aef7..13ce28081982 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1413,20 +1413,14 @@ static int map_update_elem(union bpf_attr *attr, bpfptr_t uattr)
}
value_size = bpf_map_value_size(map);
-
- err = -ENOMEM;
- value = kvmalloc(value_size, GFP_USER | __GFP_NOWARN);
- if (!value)
+ value = kvmemdup_bpfptr(uvalue, value_size);
+ if (IS_ERR(value)) {
+ err = PTR_ERR(value);
goto free_key;
-
- err = -EFAULT;
- if (copy_from_bpfptr(value, uvalue, value_size) != 0)
- goto free_value;
+ }
err = bpf_map_update_value(map, f, key, value, attr->flags);
-free_value:
- kvfree(value);
free_key:
kvfree(key);
err_put:
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [bpf-next] bpf: use kvmemdup_bpfptr helper
2022-09-09 10:58 [bpf-next] bpf: use kvmemdup_bpfptr helper Wang Yufen
@ 2022-09-09 17:27 ` sdf
2022-09-13 4:24 ` wangyufen
0 siblings, 1 reply; 3+ messages in thread
From: sdf @ 2022-09-09 17:27 UTC (permalink / raw)
To: Wang Yufen
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, haoluo, jolsa, bpf, linux-kernel
On 09/09, Wang Yufen wrote:
> Use kvmemdup_bpfptr helper instead of open-coding to
> simplify the code.
> Signed-off-by: Wang Yufen <wangyufen@huawei.com>
> ---
> kernel/bpf/syscall.c | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 4e9d4622aef7..13ce28081982 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1413,20 +1413,14 @@ static int map_update_elem(union bpf_attr *attr,
> bpfptr_t uattr)
> }
> value_size = bpf_map_value_size(map);
> -
> - err = -ENOMEM;
> - value = kvmalloc(value_size, GFP_USER | __GFP_NOWARN);
> - if (!value)
> + value = kvmemdup_bpfptr(uvalue, value_size);
> + if (IS_ERR(value)) {
> + err = PTR_ERR(value);
> goto free_key;
> -
> - err = -EFAULT;
> - if (copy_from_bpfptr(value, uvalue, value_size) != 0)
> - goto free_value;
> + }
> err = bpf_map_update_value(map, f, key, value, attr->flags);
[..]
> -free_value:
> - kvfree(value);
And here you leak the value. We need to free it after update regardless
of error/success. That's why it is coded like that.
> free_key:
> kvfree(key);
> err_put:
> --
> 2.25.1
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [bpf-next] bpf: use kvmemdup_bpfptr helper
2022-09-09 17:27 ` sdf
@ 2022-09-13 4:24 ` wangyufen
0 siblings, 0 replies; 3+ messages in thread
From: wangyufen @ 2022-09-13 4:24 UTC (permalink / raw)
To: sdf
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, haoluo, jolsa, bpf, linux-kernel
在 2022/9/10 1:27, sdf@google.com 写道:
> On 09/09, Wang Yufen wrote:
>> Use kvmemdup_bpfptr helper instead of open-coding to
>> simplify the code.
>
>> Signed-off-by: Wang Yufen <wangyufen@huawei.com>
>> ---
>> kernel/bpf/syscall.c | 14 ++++----------
>> 1 file changed, 4 insertions(+), 10 deletions(-)
>
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index 4e9d4622aef7..13ce28081982 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -1413,20 +1413,14 @@ static int map_update_elem(union bpf_attr
>> *attr, bpfptr_t uattr)
>> }
>
>> value_size = bpf_map_value_size(map);
>> -
>> - err = -ENOMEM;
>> - value = kvmalloc(value_size, GFP_USER | __GFP_NOWARN);
>> - if (!value)
>> + value = kvmemdup_bpfptr(uvalue, value_size);
>> + if (IS_ERR(value)) {
>> + err = PTR_ERR(value);
>> goto free_key;
>> -
>> - err = -EFAULT;
>> - if (copy_from_bpfptr(value, uvalue, value_size) != 0)
>> - goto free_value;
>> + }
>
>> err = bpf_map_update_value(map, f, key, value, attr->flags);
>
>
> [..]
>
>> -free_value:
>> - kvfree(value);
>
> And here you leak the value. We need to free it after update regardless
> of error/success. That's why it is coded like that.
Thanks for your comments.
It's my mistake.
Can I keep kvfree(value); and send v2?
>
>> free_key:
>> kvfree(key);
>> err_put:
>> --
>> 2.25.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-09-13 4:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-09 10:58 [bpf-next] bpf: use kvmemdup_bpfptr helper Wang Yufen
2022-09-09 17:27 ` sdf
2022-09-13 4:24 ` wangyufen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox