| 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
| ||
|
Message-ID: <CA+EESO6VgQaHqZq9earDSMyxg2pqjPtUAL74ndN8TUOnLY=c7g@mail.gmail.com>
Date: Thu, 22 May 2025 17:03:53 -0700
From: Lokesh Gidra <lokeshgidra@...gle.com>
To: Barry Song <21cnbao@...il.com>
Cc: Peter Xu <peterx@...hat.com>, David Hildenbrand <david@...hat.com>,
Suren Baghdasaryan <surenb@...gle.com>, Andrea Arcangeli <aarcange@...hat.com>,
Andrew Morton <akpm@...ux-foundation.org>, Linux-MM <linux-mm@...ck.org>,
Kairui Song <ryncsn@...il.com>, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [BUG]userfaultfd_move fails to move a folio when swap-in occurs
concurrently with swap-out
On Thu, May 22, 2025 at 4:53 PM Barry Song <21cnbao@...il.com> wrote:
>
> On Fri, May 23, 2025 at 11:44 AM Lokesh Gidra <lokeshgidra@...gle.com> wrote:
> >
> > Thanks Barry for stress testing MOVE ioctl. It's really helpful :)
> >
> > On Thu, May 22, 2025 at 4:23 PM Barry Song <21cnbao@...il.com> wrote:
> > >
> > > Hi All,
> > >
> > > I'm encountering another bug that can be easily reproduced using the small
> > > program below[1], which performs swap-out and swap-in in parallel.
> > >
> > > The issue occurs when a folio is being swapped out while it is accessed
> > > concurrently. In this case, do_swap_page() handles the access. However,
> > > because the folio is under writeback, do_swap_page() completely removes
> > > its exclusive attribute.
> > >
> > > do_swap_page:
> > > } else if (exclusive && folio_test_writeback(folio) &&
> > > data_race(si->flags & SWP_STABLE_WRITES)) {
> > > ...
> > > exclusive = false;
> > >
> > > As a result, userfaultfd_move() will return -EBUSY, even though the
> > > folio is not shared and is in fact exclusively owned.
> > >
> > > folio = vm_normal_folio(src_vma, src_addr,
> > > orig_src_pte);
> > > if (!folio || !PageAnonExclusive(&folio->page)) {
> > > spin_unlock(src_ptl);
> > > + pr_err("%s %d folio:%lx exclusive:%d
> > > swapcache:%d\n",
> > > + __func__, __LINE__, folio,
> > > PageAnonExclusive(&folio->page),
> > > + folio_test_swapcache(folio));
> > > err = -EBUSY;
> > > goto out;
> > > }
> > >
> > > I understand that shared folios should not be moved. However, in this
> > > case, the folio is not shared, yet its exclusive flag is not set.
> > >
> > > Therefore, I believe PageAnonExclusive is not a reliable indicator of
> > > whether a folio is truly exclusive to a process.
> > >
> > > The kernel log output is shown below:
> > > [ 23.009516] move_pages_pte 1285 folio:fffffdffc01bba40 exclusive:0
> > > swapcache:1
> > >
> > > I'm still struggling to find a real fix; it seems quite challenging.
> > > Please let me know if you have any ideas. In any case It seems
> > > userspace should fall back to userfaultfd_copy.
> > >
> > I'm not sure this is really a bug. A page under write-back is in a way
> > 'busy' isn't it? I am not an expert of anon-exclusive, but it seems to
> > me that an exclusively mapped anonymous page would have it true. So,
> > isn't it expected that a page under write-back will not have it set as
> > the page isn't mapped?
>
> We have two return codes: -EAGAIN and -EBUSY. In many cases, we return
> -EAGAIN, which is transparent to userspace because the syscall is retried.
>
> Therefore, I expect -EAGAIN or a similar code here to avoid userspace noise,
> since we handle other cases where folios are undergoing transitions to
> become stable again by -EAGAIN.
I agree, this case probably should be -EAGAIN. OTOH, with -EBUSY, we
know that a userfaultfd_copy fallback or some other operation needs to
be performed before retrying. In most of the -EAGAIN cases, we simply
have to retry without taking any additional steps.
>
> >
> > I have observed this in my testing as well, and there are a couple of
> > ways to deal with it in userspace. As you suggested, falling back to
> > userfaultfd_copy on receiving -EBUSY is one option. In my case, making
> > a fake store on the src page and then retrying has been working fine.
>
> Good to know you have some fallbacks implemented in userspace.
> That makes the issue less serious now.
I have been assuming so far that the reason I'm getting -EBUSY is
because somehow the page is COW shared with zygote. But what you
pointed out makes a lot more sense.
>
> > >
> > >
> > > [1] The small program:
> > >
> > > //Just in a couple of seconds, we are running into
> > > //"UFFDIO_MOVE: Device or resource busy"
> > >
> > > #define _GNU_SOURCE
> > > #include <stdio.h>
> > > #include <stdlib.h>
> > > #include <string.h>
> > > #include <sys/mman.h>
> > > #include <sys/ioctl.h>
> > > #include <sys/syscall.h>
> > > #include <linux/userfaultfd.h>
> > > #include <fcntl.h>
> > > #include <pthread.h>
> > > #include <unistd.h>
> > > #include <poll.h>
> > > #include <errno.h>
> > >
> > > #define PAGE_SIZE 4096
> > > #define REGION_SIZE (512 * 1024)
> > >
> > > #ifndef UFFDIO_MOVE
> > > struct uffdio_move {
> > > __u64 dst;
> > > __u64 src;
> > > __u64 len;
> > > #define UFFDIO_MOVE_MODE_DONTWAKE ((__u64)1<<0)
> > > #define UFFDIO_MOVE_MODE_ALLOW_SRC_HOLES ((__u64)1<<1)
> > > __u64 mode;
> > > __s64 move;
> > > };
> > >
> > > #define _UFFDIO_MOVE (0x05)
> > > #define UFFDIO_MOVE _IOWR(UFFDIO, _UFFDIO_MOVE, struct uffdio_move)
> > > #endif
> > >
> > >
> > > void *src, *dst;
> > > int uffd;
> > >
> > > void *madvise_thread(void *arg) {
> > > for (size_t i = 0; i < REGION_SIZE; i += PAGE_SIZE) {
> > > madvise(src + i, PAGE_SIZE, MADV_PAGEOUT);
> > > usleep(100);
> > > }
> > > return NULL;
> > > }
> > >
> > > void *swapin_thread(void *arg) {
> > > volatile char dummy;
> > > for (size_t i = 0; i < REGION_SIZE; i += PAGE_SIZE) {
> > > dummy = ((char *)src)[i];
> > > usleep(100);
> > > }
> > > return NULL;
> > > }
> > >
> > >
> > > void *fault_handler_thread(void *arg) {
> > >
> > > struct uffd_msg msg;
> > > struct uffdio_move move;
> > > struct pollfd pollfd = { .fd = uffd, .events = POLLIN };
> > > pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL);
> > > pthread_setcanceltype(PTHREAD_CANCEL_DEFERRED, NULL);
> > >
> > > while (1) {
> > > if (poll(&pollfd, 1, -1) == -1) {
> > > perror("poll");
> > > exit(EXIT_FAILURE);
> > > }
> > >
> > > if (read(uffd, &msg, sizeof(msg)) <= 0) {
> > > perror("read");
> > > exit(EXIT_FAILURE);
> > > }
> > >
> > >
> > > if (msg.event != UFFD_EVENT_PAGEFAULT) {
> > > fprintf(stderr, "Unexpected event\n");
> > > exit(EXIT_FAILURE);
> > > }
> > >
> > > move.src = (unsigned long)src + (msg.arg.pagefault.address -
> > > (unsigned long)dst);
> > > move.dst = msg.arg.pagefault.address & ~(PAGE_SIZE - 1);
> > > move.len = PAGE_SIZE;
> > > move.mode = 0;
> > >
> > > if (ioctl(uffd, UFFDIO_MOVE, &move) == -1) {
> > > perror("UFFDIO_MOVE");
> > > exit(EXIT_FAILURE);
> > > }
> > > }
> > > return NULL;
> > > }
> > >
> > > int main() {
> > > again:
> > > pthread_t thr, madv_thr, swapin_thr;
> > > struct uffdio_api uffdio_api = { .api = UFFD_API, .features = 0 };
> > > struct uffdio_register uffdio_register;
> > >
> > > src = mmap(NULL, REGION_SIZE, PROT_READ | PROT_WRITE, MAP_PRIVATE
> > > | MAP_ANONYMOUS, -1, 0);
> > >
> > > if (src == MAP_FAILED) {
> > > perror("mmap src");
> > > exit(EXIT_FAILURE);
> > > }
> > >
> > > memset(src, 1, REGION_SIZE);
> > >
> > > dst = mmap(NULL, REGION_SIZE, PROT_READ | PROT_WRITE, MAP_PRIVATE
> > > | MAP_ANONYMOUS, -1, 0);
> > >
> > > if (dst == MAP_FAILED) {
> > > perror("mmap dst");
> > > exit(EXIT_FAILURE);
> > > }
> > >
> > >
> > > uffd = syscall(SYS_userfaultfd, O_CLOEXEC | O_NONBLOCK);
> > > if (uffd == -1) {
> > > perror("userfaultfd");
> > > exit(EXIT_FAILURE);
> > > }
> > >
> > >
> > > if (ioctl(uffd, UFFDIO_API, &uffdio_api) == -1) {
> > > perror("UFFDIO_API");
> > > exit(EXIT_FAILURE);
> > > }
> > >
> > > uffdio_register.range.start = (unsigned long)dst;
> > > uffdio_register.range.len = REGION_SIZE;
> > > uffdio_register.mode = UFFDIO_REGISTER_MODE_MISSING;
> > >
> > > if (ioctl(uffd, UFFDIO_REGISTER, &uffdio_register) == -1) {
> > > perror("UFFDIO_REGISTER");
> > > exit(EXIT_FAILURE);
> > >
> > > }
> > >
> > > if (pthread_create(&madv_thr, NULL, madvise_thread, NULL) != 0) {
> > > perror("pthread_create madvise_thread");
> > > exit(EXIT_FAILURE);
> > > }
> > >
> > > if (pthread_create(&swapin_thr, NULL, swapin_thread, NULL) != 0) {
> > > perror("pthread_create swapin_thread");
> > > exit(EXIT_FAILURE);
> > > }
> > >
> > > if (pthread_create(&thr, NULL, fault_handler_thread, NULL) != 0) {
> > > perror("pthread_create fault_handler_thread");
> > > exit(EXIT_FAILURE);
> > > }
> > >
> > > for (size_t i = 0; i < REGION_SIZE; i += PAGE_SIZE) {
> > > char val = ((char *)dst)[i];
> > > printf("Accessing dst at offset %zu, value: %d\n", i, val);
> > > }
> > >
> > > pthread_join(madv_thr, NULL);
> > > pthread_join(swapin_thr, NULL);
> > > pthread_cancel(thr);
> > > pthread_join(thr, NULL);
> > > munmap(src, REGION_SIZE);
> > > munmap(dst, REGION_SIZE);
> > > close(uffd);
> > > goto again;
> > >
> > > return 0;
> > > }
> > >
>
> Thanks
> Barry
Powered by blists - more mailing lists