All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Dmitry Vyukov <dvyukov@google.com>
Cc: Alexander Potapenko <glider@google.com>,
	Kostya Serebryany <kcc@google.com>,
	Eric Dumazet <edumazet@google.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] [iov_iter] use memmove() when copying to/from user page
Date: Tue, 16 May 2017 20:37:50 +0100	[thread overview]
Message-ID: <20170516193750.GR390@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CACT4Y+YKvbwK=8hs2BD8gvR=hu6H70WdJcBOFKnXwfJwPUTgzA@mail.gmail.com>

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@zeniv.linux.org.uk> 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.

  reply	other threads:[~2017-05-16 19:37 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-16 12:27 [PATCH] [iov_iter] use memmove() when copying to/from user page Alexander Potapenko
2017-05-16 18:48 ` Al Viro
2017-05-16 18:53   ` Dmitry Vyukov
2017-05-16 19:37     ` Al Viro [this message]
2017-05-16 20:10       ` Dmitry Vyukov
2017-05-16 20:52         ` Al Viro
2017-05-16 21:01           ` Dmitry Vyukov
2017-05-16 21:33             ` Al Viro
2017-05-16 22:15               ` Dmitry Vyukov
2017-05-16 22:48                 ` Al Viro
2017-05-25 17:04                   ` Pavel Machek
2017-05-25 17:15                     ` Eric Dumazet
2017-05-25 21:27                       ` Pavel Machek
2017-05-25 18:22                     ` Al Viro
2017-05-25 19:15                       ` Al Viro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170516193750.GR390@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=dvyukov@google.com \
    --cc=edumazet@google.com \
    --cc=glider@google.com \
    --cc=kcc@google.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.