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

[Dmitry Vyukov]

On Tue, May 16, 2017 at 1:52 PM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> On Tue, May 16, 2017 at 01:10:56PM -0700, Dmitry Vyukov wrote:
>
>> >         * 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.
>>
>> I would say it's already documented.
>> sendfile says that it "copies data". memmove says that it "copies
>> data". memcpy says that it "copies data, but data must not overlap".
>> sendfile does not say that "data must not overlap".
>
> In that case your patch does not suffice.  Overlapping move _forwards_ still
> yields unexpected results, doesn't it?

Why? memmove can move both ways. Do we need to change more memcpy's to
memmove's?


>  I'm all for documenting that
> resulting contents is undefined in case of overlap.  The same goes for write()
> from mmap'ed area, BTW.  You suggest changing that undefined behaviour *and*
> either pretending that it's not undefined anymore (obviously false)

Why is it false?

> or
> failing to describe the new cases when it is not undefined anymore.

I would say it just fixes a bug in current impl. Sendfile docs always
said "sendfile() copies data" (without any but's), but we failed to do
this.


> It's not the cost of extra branch; it's ill-defined rules that would need to
> be followed to be able to rely upon the "improvement".