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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 25 May 2017 20:15:57 +0100
From:   Al Viro <viro@...IV.linux.org.uk>
To:     Pavel Machek <pavel@....cz>
Cc:     mtk.manpages@...il.com, Dmitry Vyukov <dvyukov@...gle.com>,
        Alexander Potapenko <glider@...gle.com>,
        Kostya Serebryany <kcc@...gle.com>,
        Eric Dumazet <edumazet@...gle.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] [iov_iter] use memmove() when copying to/from user page

On Thu, May 25, 2017 at 07:22:01PM +0100, Al Viro wrote:

> It does not make overlapping sendfile() work reliably (while
> creating an impression that it just might, as we'd seen in
> this thread).  It does not do anything to kernel-vs-userland
> aliasing either - copy_from_user() is definitely memcpy()-like,
> not memmove()-like.  Not that memmove() worked in situations
> when source and destination point to the same memory object
> seen at two virtial addresses...

BTW, the last part goes both for kernel and for userland:

#include <stdio.h>
#include <fcntl.h>
#include <unistd.h>
#include <string.h>
#include <sys/mman.h>
#include <sys/stat.h>

void f(char *p, char *q)
{
	memset(p, 0, 4096);
	p[254] = 1;
        memmove(q, p + 127, 256);
        memmove(p, q + 127, 256);
	printf("%d\n", p[0]);
}

main()
{
        static char p[4096];
        int fd = creat("/tmp/foo", 0600);
        unsigned char *p1, *p2;
        int i;

        write(fd, p, 4096);
        close(fd);
        fd = open("/tmp/foo", O_RDWR);
        p1 = mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
        p2 = mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);

	f(p1, p1);
	f(p1, p2);
	f(p2, p1);
}

should've printed 1 three times, according to C99/SuS.  On
stretch/amd64 (with 2.24-10 glibc) we get
1
1
0
instead.  mmap() is out of scope for C99, of course, but not for SuS.
And SuS treatment of memmove() is:

    [CX] [Option Start] The functionality described on this reference
    page is aligned with the ISO C standard. Any conflict between
    the requirements described here and the ISO C standard is
    unintentional. This volume of POSIX.1-2008 defers to the ISO C
    standard. [Option End]

    The memmove() function shall copy n bytes from the object pointed
    to by s2 into the object pointed to by s1. Copying takes place as if
    the n bytes from the object pointed to by s2 are first copied into a
    temporary array of n bytes that does not overlap the objects pointed
    to by s1 and s2, and then the n bytes from the temporary array are
    copied into the object pointed to by s1.

In case when physical memory areas overlap but addresses do not, results
of memmove(3) are impossible to rely upon on all implementations I've
seen.