Re: [bpf-next] bpf: use kvmemdup_bpfptr helper

[wangyufen]

在 2022/9/10 1:27, sdf@xxxxxxxxxx 写道:
> On 09/09, Wang Yufen wrote:
> > Use kvmemdup_bpfptr helper instead of open-coding to
> > simplify the code.
>
> > Signed-off-by: Wang Yufen <wangyufen@xxxxxxxxxx>
> > ---
> >   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