Re: [PATCH] userfaultfd: mark uffd_wp regardless of VM_WRITE flag

[Nadav Amit <namit@xxxxxxxxxx>]

> On Feb 17, 2022, at 1:16 PM, Nadav Amit <nadav.amit@xxxxxxxxx> wrote:
> 
> From: Nadav Amit <namit@xxxxxxxxxx>
> 
> When a PTE is set by UFFD operations such as UFFDIO_COPY, the PTE is
> currently only marked as write-protected if the VMA has VM_WRITE flag
> set. This seems incorrect or at least would be unexpected by the users.

One more note - if you want you can try the following reproducer:

#define _GNU_SOURCE
#include <linux/userfaultfd.h>
#include <errno.h>
#include <unistd.h>
#include <stdlib.h>
#include <fcntl.h>
#include <pthread.h>
#include <sys/mman.h>
#include <sys/syscall.h>
#include <sys/ioctl.h>
#include <sys/mman.h>
#include <sys/types.h>
#include <stdio.h>

volatile int ok;

static void * fault_handler_thread(void *arg)
{
	struct uffd_msg msg = {0};
	int nread;
	int uffd = (int)(long)arg;
	struct uffdio_writeprotect uffd_wp = {0};

	nread = read(uffd, &msg, sizeof(msg));
	if (nread == 0) {
		printf("EOF on userfaultfd!\n");
		exit(EXIT_FAILURE);
	}

	ok = 1;

	uffd_wp.range.start = msg.arg.pagefault.address & ~(4095ull);
	uffd_wp.range.len = 4096;

	if (ioctl(uffd, UFFDIO_WRITEPROTECT, &uffd_wp) == -1) {
		perror("uffd-w-unprotect");
		exit(EXIT_FAILURE);
	}
	return NULL;
}

char page[4096];

int main(void)
{
	struct uffdio_api uffdio_api = {0};
	struct uffdio_register uffdio_register = {0};
	struct uffdio_writeprotect uffdio_wp = {0};
	struct uffdio_copy uffdio_copy = {0};
	volatile char *pc;
	pthread_t thr;
	int err;
	int uffd;
	void *p;

	uffd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
	if (uffd == -1) {
		perror("userfaultfd");
		exit(-1);
	}

	uffdio_api.api = UFFD_API;
	uffdio_api.features = UFFD_FEATURE_PAGEFAULT_FLAG_WP;
	if (ioctl(uffd, UFFDIO_API, &uffdio_api) == -1) {
		perror("api");
		exit(EXIT_FAILURE);
	}

	p = mmap(NULL, 4096, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);

	if (p == MAP_FAILED) {
		perror("mmap");
		exit(EXIT_FAILURE);
	}

	uffdio_register.range.start = (unsigned long)p;
	uffdio_register.range.len = 4096;
	uffdio_register.mode = UFFDIO_REGISTER_MODE_MISSING|UFFDIO_REGISTER_MODE_WP;
	if (ioctl(uffd, UFFDIO_REGISTER, &uffdio_register) == -1) {
		perror("register");
		exit(EXIT_FAILURE);
	}

	uffdio_copy.src = (unsigned long)page;
	uffdio_copy.dst = (unsigned long)p;
	uffdio_copy.len = 4096;
	uffdio_copy.mode = UFFDIO_COPY_MODE_WP;
	uffdio_copy.copy = 0;
	if (ioctl(uffd, UFFDIO_COPY, &uffdio_copy) == -1) {
		perror("uffd-copy");
		exit(EXIT_FAILURE);
	}

	err = pthread_create(&thr, NULL, fault_handler_thread, (void *)(long)uffd);
	if (err != 0) {
		exit(EXIT_FAILURE);
	}

	if (mprotect(p, 4096, PROT_READ|PROT_WRITE) < 0) {
		perror("mprotect(PROT_READ|PROT_WRITE)");
		exit(EXIT_FAILURE);
	}

	pc = (volatile char *)p;
	*pc = 1;
	printf("%s\n", ok ? "ok" : "failed”);
	return 0;
}