Re: [PATCH] [iov_iter] use memmove() when copying to/from user page

From: Al Viro
Date: Tue May 16 2017 - 15:37:55 EST


On Tue, May 16, 2017 at 11:53:01AM -0700, Dmitry Vyukov wrote:
> On Tue, May 16, 2017 at 11:48 AM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> > On Tue, May 16, 2017 at 02:27:34PM +0200, Alexander Potapenko wrote:
> >> It's possible that calling sendfile() to copy the data from a memfd to
> >> itself may result in doing a memcpy() with overlapping arguments.
> >> To avoid undefined behavior here, replace memcpy() with memmove() and
> >> rename memcpy_to_page()/memcpy_from_page() accordingly.
> >
> > Er... And what semantics would you assign to such sendfile()? I really
> > want to see details, because it sounds like memmove() here will not be
> > any more useful than memcpy() - you still can esily get odd behaviour.
>
>
> What odd behavior can we get with memmove?
>
> Case that I am thinking of is when you want to delete part of the file
> in the middle. To do that you move tail of the file and then truncate.
> Memmove will do the intended thing. While memcpy can lost of data and
> duplicate another.

Oh, lovely. While we are trading idiotic use cases - what about inserting
something in the middle of a file? No? Why is it any different?

There are two sides to it:
* real nasal demons resulting from that memcpy() with overlapping
source and destination - as in, "it not only trashed the page contents,
it has led to memory corruption/leaked data/etc". Any such would be a real
problem.
* behaviour of sendfile() in such a case. And there I've no problem
with saying "contents after operation is undefined". If you wish to change
that, by all means start with documenting the semantics you want to promise
to userland.