• Peter Xu's avatar
    mm/mprotect: fix soft-dirty check in can_change_pte_writable() · 76aefad6
    Peter Xu authored
    Patch series "mm/mprotect: Fix soft-dirty checks", v4.
    
    
    This patch (of 3):
    
    The check wanted to make sure when soft-dirty tracking is enabled we won't
    grant write bit by accident, as a page fault is needed for dirty tracking.
    The intention is correct but we didn't check it right because
    VM_SOFTDIRTY set actually means soft-dirty tracking disabled.  Fix it.
    
    There's another thing tricky about soft-dirty is that, we can't check the
    vma flag !(vma_flags & VM_SOFTDIRTY) directly but only check it after we
    checked CONFIG_MEM_SOFT_DIRTY because otherwise VM_SOFTDIRTY will be
    defined as zero, and !(vma_flags & VM_SOFTDIRTY) will constantly return
    true.  To avoid misuse, introduce a helper for checking whether vma has
    soft-dirty tracking enabled.
    
    We can easily verify this with any exclusive anonymous page, like program
    below:
    
    =======8<======
      #include <stdio.h>
      #include <unistd.h>
      #include <stdlib.h>
      #include <assert.h>
      #include <inttypes.h>
      #include <stdint.h>
      #include <sys/types.h>
      #include <sys/mman.h>
      #include <sys/types.h>
      #include <sys/stat.h>
      #include <unistd.h>
      #include <fcntl.h>
      #include <stdbool.h>
    
      #define BIT_ULL(nr)                   (1ULL << (nr))
      #define PM_SOFT_DIRTY                 BIT_ULL(55)
    
      unsigned int psize;
      char *page;
    
      uint64_t pagemap_read_vaddr(int fd, void *vaddr)
      {
          uint64_t value;
          int ret;
    
          ret = pread(fd, &value, sizeof(uint64_t),
                      ((uint64_t)vaddr >> 12) * sizeof(uint64_t));
          assert(ret == sizeof(uint64_t));
    
          return value;
      }
    
      void clear_refs_write(void)
      {
          int fd = open("/proc/self/clear_refs", O_RDWR);
    
          assert(fd >= 0);
          write(fd, "4", 2);
          close(fd);
      }
    
      #define  check_soft_dirty(str, expect)  do {                            \
              bool dirty = pagemap_read_vaddr(fd, page) & PM_SOFT_DIRTY;      \
              if (dirty != expect) {                                          \
                  printf("ERROR: %s, soft-dirty=%d (expect: %d)
    ", str, dirty, expect); \
                  exit(-1);                                                   \
              }                                                               \
      } while (0)
    
      int main(void)
      {
          int fd = open("/proc/self/pagemap", O_RDONLY);
    
          assert(fd >= 0);
          psize = getpagesize();
          page = mmap(NULL, psize, PROT_READ|PROT_WRITE,
                      MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
          assert(page != MAP_FAILED);
    
          *page = 1;
          check_soft_dirty("Just faulted in page", 1);
          clear_refs_write();
          check_soft_dirty("Clear_refs written", 0);
          mprotect(page, psize, PROT_READ);
          check_soft_dirty("Marked RO", 0);
          mprotect(page, psize, PROT_READ|PROT_WRITE);
          check_soft_dirty("Marked RW", 0);
          *page = 2;
          check_soft_dirty("Wrote page again", 1);
    
          munmap(page, psize);
          close(fd);
          printf("Test passed.
    ");
    
          return 0;
      }
    =======8<======
    
    Here we attach a Fixes to commit 64fe24a3 only for easy tracking, as
    this patch won't apply to a tree before that point.  However the commit
    wasn't the source of problem, but instead 64e45507.  It's just that
    after 64fe24a3 anonymous memory will also suffer from this problem
    with mprotect().
    
    Link: https://lkml.kernel.org/r/20220725142048.30450-1-peterx@redhat.com
    Link: https://lkml.kernel.org/r/20220725142048.30450-2-peterx@redhat.com
    Fixes: 64e45507 ("mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared")
    Fixes: 64fe24a3 ("mm/mprotect: try avoiding write faults for exclusive anonymous pages when changing protection")
    Signed-off-by: default avatarPeter Xu <peterx@redhat.com>
    Reviewed-by: default avatarDavid Hildenbrand <david@redhat.com>
    Cc: Nadav Amit <nadav.amit@gmail.com>
    Cc: Andrea Arcangeli <aarcange@redhat.com>
    Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
    76aefad6
mmap.c 100 KB