Re: [PATCH bpf-next v1 6/7] bpf: Use memmove for bpf_dynptr_{read,write}

[David Vernet <void@manifault.com>]

On Tue, Nov 15, 2022 at 05:31:29AM +0530, Kumar Kartikeya Dwivedi wrote:
> It may happen that destination buffer memory overlaps with memory dynptr
> points to. Hence, we must use memmove to correctly copy from dynptr to
> destination buffer, or source buffer to dynptr.
> 
> This actually isn't a problem right now, as memcpy implementation falls
> back to memmove on detecting overlap and warns about it, but we
> shouldn't be relying on that.
> 
> Acked-by: Joanne Koong <joannelkoong@gmail.com>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---

Good call, thanks for this.

Acked-by: David Vernet <void@manifault.com>

>  kernel/bpf/helpers.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 714f5c9d0c1f..1099ed1e7712 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -1489,7 +1489,7 @@ BPF_CALL_5(bpf_dynptr_read, void *, dst, u32, len, const struct bpf_dynptr_kern
>  	if (err)
>  		return err;
>  
> -	memcpy(dst, src->data + src->offset + offset, len);
> +	memmove(dst, src->data + src->offset + offset, len);

Can you add a comment here and in bpf_dynptr_write() which explain why
memmove is needed?

>  
>  	return 0;
>  }
> @@ -1517,7 +1517,7 @@ BPF_CALL_5(bpf_dynptr_write, const struct bpf_dynptr_kern *, dst, u32, offset, v
>  	if (err)
>  		return err;
>  
> -	memcpy(dst->data + dst->offset + offset, src, len);
> +	memmove(dst->data + dst->offset + offset, src, len);
>  
>  	return 0;
>  }
> -- 
> 2.38.1
>