lkml.org 
[lkml]   [2021]   [Jul]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v5 17/26] hugetlb/userfaultfd: Hook page faults for uffd write protection
    Hi Peter,

    Thank you for the patch! Yet something to improve:

    [auto build test ERROR on kselftest/next]
    [also build test ERROR on linus/master v5.14-rc2 next-20210720]
    [cannot apply to hnaz-linux-mm/master asm-generic/master arm64/for-next/core linux/master tip/x86/core]
    [If your patch is applied to the wrong git tree, kindly drop us a note.
    And when submitting patch, we suggest to use '--base' as documented in
    https://git-scm.com/docs/git-format-patch]

    url: https://github.com/0day-ci/linux/commits/Peter-Xu/userfaultfd-wp-Support-shmem-and-hugetlbfs/20210716-041947
    base: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git next
    config: s390-randconfig-r023-20210716 (attached as .config)
    compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 5d5b08761f944d5b9822d582378333cc4b36a0a7)
    reproduce (this is a W=1 build):
    wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
    chmod +x ~/bin/make.cross
    # install s390 cross compiling tool for clang build
    # apt-get install binutils-s390x-linux-gnu
    # https://github.com/0day-ci/linux/commit/071935856c8e636cafde633db59259d75069cc8f
    git remote add linux-review https://github.com/0day-ci/linux
    git fetch --no-tags linux-review Peter-Xu/userfaultfd-wp-Support-shmem-and-hugetlbfs/20210716-041947
    git checkout 071935856c8e636cafde633db59259d75069cc8f
    # save the attached .config to linux build tree
    COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390

    If you fix the issue, kindly add following tag as appropriate
    Reported-by: kernel test robot <lkp@intel.com>

    All errors (new ones prefixed by >>):

    In file included from mm/hugetlb.c:19:
    In file included from include/linux/memblock.h:14:
    In file included from arch/s390/include/asm/dma.h:5:
    In file included from arch/s390/include/asm/io.h:75:
    include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
    val = __raw_readb(PCI_IOBASE + addr);
    ~~~~~~~~~~ ^
    include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
    val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
    ~~~~~~~~~~ ^
    include/uapi/linux/byteorder/big_endian.h:36:59: note: expanded from macro '__le16_to_cpu'
    #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
    ^
    include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
    #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
    ^
    In file included from mm/hugetlb.c:19:
    In file included from include/linux/memblock.h:14:
    In file included from arch/s390/include/asm/dma.h:5:
    In file included from arch/s390/include/asm/io.h:75:
    include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
    val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
    ~~~~~~~~~~ ^
    include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
    #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
    ^
    include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
    #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
    ^
    In file included from mm/hugetlb.c:19:
    In file included from include/linux/memblock.h:14:
    In file included from arch/s390/include/asm/dma.h:5:
    In file included from arch/s390/include/asm/io.h:75:
    include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
    __raw_writeb(value, PCI_IOBASE + addr);
    ~~~~~~~~~~ ^
    include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
    __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
    ~~~~~~~~~~ ^
    include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
    __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
    ~~~~~~~~~~ ^
    include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
    readsb(PCI_IOBASE + addr, buffer, count);
    ~~~~~~~~~~ ^
    include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
    readsw(PCI_IOBASE + addr, buffer, count);
    ~~~~~~~~~~ ^
    include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
    readsl(PCI_IOBASE + addr, buffer, count);
    ~~~~~~~~~~ ^
    include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
    writesb(PCI_IOBASE + addr, buffer, count);
    ~~~~~~~~~~ ^
    include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
    writesw(PCI_IOBASE + addr, buffer, count);
    ~~~~~~~~~~ ^
    include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
    writesl(PCI_IOBASE + addr, buffer, count);
    ~~~~~~~~~~ ^
    >> mm/hugetlb.c:5063:29: error: implicit declaration of function 'huge_pte_uffd_wp' [-Werror,-Wimplicit-function-declaration]
    if (userfaultfd_wp(vma) && huge_pte_uffd_wp(huge_ptep_get(ptep)) &&
    ^
    12 warnings and 1 error generated.


    vim +/huge_pte_uffd_wp +5063 mm/hugetlb.c

    4957
    4958 vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
    4959 unsigned long address, unsigned int flags)
    4960 {
    4961 pte_t *ptep, entry;
    4962 spinlock_t *ptl;
    4963 vm_fault_t ret;
    4964 u32 hash;
    4965 pgoff_t idx;
    4966 struct page *page = NULL;
    4967 struct page *pagecache_page = NULL;
    4968 struct hstate *h = hstate_vma(vma);
    4969 struct address_space *mapping;
    4970 int need_wait_lock = 0;
    4971 unsigned long haddr = address & huge_page_mask(h);
    4972
    4973 ptep = huge_pte_offset(mm, haddr, huge_page_size(h));
    4974 if (ptep) {
    4975 /*
    4976 * Since we hold no locks, ptep could be stale. That is
    4977 * OK as we are only making decisions based on content and
    4978 * not actually modifying content here.
    4979 */
    4980 entry = huge_ptep_get(ptep);
    4981 if (unlikely(is_hugetlb_entry_migration(entry))) {
    4982 migration_entry_wait_huge(vma, mm, ptep);
    4983 return 0;
    4984 } else if (unlikely(is_hugetlb_entry_hwpoisoned(entry)))
    4985 return VM_FAULT_HWPOISON_LARGE |
    4986 VM_FAULT_SET_HINDEX(hstate_index(h));
    4987 }
    4988
    4989 /*
    4990 * Acquire i_mmap_rwsem before calling huge_pte_alloc and hold
    4991 * until finished with ptep. This serves two purposes:
    4992 * 1) It prevents huge_pmd_unshare from being called elsewhere
    4993 * and making the ptep no longer valid.
    4994 * 2) It synchronizes us with i_size modifications during truncation.
    4995 *
    4996 * ptep could have already be assigned via huge_pte_offset. That
    4997 * is OK, as huge_pte_alloc will return the same value unless
    4998 * something has changed.
    4999 */
    5000 mapping = vma->vm_file->f_mapping;
    5001 i_mmap_lock_read(mapping);
    5002 ptep = huge_pte_alloc(mm, vma, haddr, huge_page_size(h));
    5003 if (!ptep) {
    5004 i_mmap_unlock_read(mapping);
    5005 return VM_FAULT_OOM;
    5006 }
    5007
    5008 /*
    5009 * Serialize hugepage allocation and instantiation, so that we don't
    5010 * get spurious allocation failures if two CPUs race to instantiate
    5011 * the same page in the page cache.
    5012 */
    5013 idx = vma_hugecache_offset(h, vma, haddr);
    5014 hash = hugetlb_fault_mutex_hash(mapping, idx);
    5015 mutex_lock(&hugetlb_fault_mutex_table[hash]);
    5016
    5017 entry = huge_ptep_get(ptep);
    5018 if (huge_pte_none(entry)) {
    5019 ret = hugetlb_no_page(mm, vma, mapping, idx, address, ptep, flags);
    5020 goto out_mutex;
    5021 }
    5022
    5023 ret = 0;
    5024
    5025 /*
    5026 * entry could be a migration/hwpoison entry at this point, so this
    5027 * check prevents the kernel from going below assuming that we have
    5028 * an active hugepage in pagecache. This goto expects the 2nd page
    5029 * fault, and is_hugetlb_entry_(migration|hwpoisoned) check will
    5030 * properly handle it.
    5031 */
    5032 if (!pte_present(entry))
    5033 goto out_mutex;
    5034
    5035 /*
    5036 * If we are going to COW the mapping later, we examine the pending
    5037 * reservations for this page now. This will ensure that any
    5038 * allocations necessary to record that reservation occur outside the
    5039 * spinlock. For private mappings, we also lookup the pagecache
    5040 * page now as it is used to determine if a reservation has been
    5041 * consumed.
    5042 */
    5043 if ((flags & FAULT_FLAG_WRITE) && !huge_pte_write(entry)) {
    5044 if (vma_needs_reservation(h, vma, haddr) < 0) {
    5045 ret = VM_FAULT_OOM;
    5046 goto out_mutex;
    5047 }
    5048 /* Just decrements count, does not deallocate */
    5049 vma_end_reservation(h, vma, haddr);
    5050
    5051 if (!(vma->vm_flags & VM_MAYSHARE))
    5052 pagecache_page = hugetlbfs_pagecache_page(h,
    5053 vma, haddr);
    5054 }
    5055
    5056 ptl = huge_pte_lock(h, mm, ptep);
    5057
    5058 /* Check for a racing update before calling hugetlb_cow */
    5059 if (unlikely(!pte_same(entry, huge_ptep_get(ptep))))
    5060 goto out_ptl;
    5061
    5062 /* Handle userfault-wp first, before trying to lock more pages */
    > 5063 if (userfaultfd_wp(vma) && huge_pte_uffd_wp(huge_ptep_get(ptep)) &&
    5064 (flags & FAULT_FLAG_WRITE) && !huge_pte_write(entry)) {
    5065 struct vm_fault vmf = {
    5066 .vma = vma,
    5067 .address = haddr,
    5068 .flags = flags,
    5069 };
    5070
    5071 spin_unlock(ptl);
    5072 if (pagecache_page) {
    5073 unlock_page(pagecache_page);
    5074 put_page(pagecache_page);
    5075 }
    5076 mutex_unlock(&hugetlb_fault_mutex_table[hash]);
    5077 i_mmap_unlock_read(mapping);
    5078 return handle_userfault(&vmf, VM_UFFD_WP);
    5079 }
    5080
    5081 /*
    5082 * hugetlb_cow() requires page locks of pte_page(entry) and
    5083 * pagecache_page, so here we need take the former one
    5084 * when page != pagecache_page or !pagecache_page.
    5085 */
    5086 page = pte_page(entry);
    5087 if (page != pagecache_page)
    5088 if (!trylock_page(page)) {
    5089 need_wait_lock = 1;
    5090 goto out_ptl;
    5091 }
    5092
    5093 get_page(page);
    5094
    5095 if (flags & FAULT_FLAG_WRITE) {
    5096 if (!huge_pte_write(entry)) {
    5097 ret = hugetlb_cow(mm, vma, address, ptep,
    5098 pagecache_page, ptl);
    5099 goto out_put_page;
    5100 }
    5101 entry = huge_pte_mkdirty(entry);
    5102 }
    5103 entry = pte_mkyoung(entry);
    5104 if (huge_ptep_set_access_flags(vma, haddr, ptep, entry,
    5105 flags & FAULT_FLAG_WRITE))
    5106 update_mmu_cache(vma, haddr, ptep);
    5107 out_put_page:
    5108 if (page != pagecache_page)
    5109 unlock_page(page);
    5110 put_page(page);
    5111 out_ptl:
    5112 spin_unlock(ptl);
    5113
    5114 if (pagecache_page) {
    5115 unlock_page(pagecache_page);
    5116 put_page(pagecache_page);
    5117 }
    5118 out_mutex:
    5119 mutex_unlock(&hugetlb_fault_mutex_table[hash]);
    5120 i_mmap_unlock_read(mapping);
    5121 /*
    5122 * Generally it's safe to hold refcount during waiting page lock. But
    5123 * here we just wait to defer the next page fault to avoid busy loop and
    5124 * the page is not used after unlocked before returning from the current
    5125 * page fault. So we are safe from accessing freed page, even if we wait
    5126 * here without taking refcount.
    5127 */
    5128 if (need_wait_lock)
    5129 wait_on_page_locked(page);
    5130 return ret;
    5131 }
    5132

    ---
    0-DAY CI Kernel Test Service, Intel Corporation
    https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
    [unhandled content-type:application/gzip]
    \
     
     \ /
      Last update: 2021-07-20 17:45    [W:3.605 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site