Re: [PATCH v5 17/26] hugetlb/userfaultfd: Hook page faults for uffd write protection

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>

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@xxxxxxxxxxxx

Attachment: .config.gz
Description: application/gzip


[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux