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
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAJHvVciSP9QyF33GFveESFW3o7vyxbydq2vR4t7tnunJLJNjWg@mail.gmail.com>
Date:   Wed, 8 Mar 2023 10:48:11 -0800
From:   Axel Rasmussen <axelrasmussen@...gle.com>
To:     kernel test robot <lkp@...el.com>
Cc:     Alexander Viro <viro@...iv.linux.org.uk>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Hugh Dickins <hughd@...gle.com>, Jan Kara <jack@...e.cz>,
        "Liam R. Howlett" <Liam.Howlett@...cle.com>,
        Matthew Wilcox <willy@...radead.org>,
        Mike Kravetz <mike.kravetz@...cle.com>,
        Mike Rapoport <rppt@...nel.org>,
        Muchun Song <muchun.song@...ux.dev>,
        Nadav Amit <namit@...are.com>, Peter Xu <peterx@...hat.com>,
        Shuah Khan <skhan@...uxfoundation.org>, llvm@...ts.linux.dev,
        oe-kbuild-all@...ts.linux.dev,
        Linux Memory Management List <linux-mm@...ck.org>,
        James Houghton <jthoughton@...gle.com>,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-kselftest@...r.kernel.org
Subject: Re: [PATCH v3 4/5] mm: userfaultfd: don't separate addr + len arguments

On Wed, Mar 8, 2023 at 1:52 AM kernel test robot <lkp@...el.com> wrote:
>
> Hi Axel,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on v6.3-rc1]
> [cannot apply to akpm-mm/mm-everything next-20230308]
> [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#_base_tree_information]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Axel-Rasmussen/mm-userfaultfd-rename-functions-for-clarity-consistency/20230307-065203
> patch link:    https://lore.kernel.org/r/20230306225024.264858-5-axelrasmussen%40google.com
> patch subject: [PATCH v3 4/5] mm: userfaultfd: don't separate addr + len arguments
> config: x86_64-randconfig-a011-20230306 (https://download.01.org/0day-ci/archive/20230308/202303081703.nwxAgIVH-lkp@intel.com/config)
> compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
> 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
>         # https://github.com/intel-lab-lkp/linux/commit/cee642b93be3ae01c7cc737c0176cbc16074a25a
>         git remote add linux-review https://github.com/intel-lab-lkp/linux
>         git fetch --no-tags linux-review Axel-Rasmussen/mm-userfaultfd-rename-functions-for-clarity-consistency/20230307-065203
>         git checkout cee642b93be3ae01c7cc737c0176cbc16074a25a
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@...el.com>
> | Link: https://lore.kernel.org/oe-kbuild-all/202303081703.nwxAgIVH-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
> >> mm/userfaultfd.c:577:52: error: passing 'const struct uffdio_range *' to parameter of incompatible type 'struct uffdio_range'
>                    return  mfill_atomic_hugetlb(dst_vma, src_start, dst, flags);
>                                                                     ^~~
>    mm/userfaultfd.c:463:29: note: passing argument to parameter 'dst' here
>                                        struct uffdio_range dst,
>                                                            ^
>    1 error generated.

Whoops. :) I admittedly didn't test with !CONFIG_HUGETLB_PAGE.

The next version of this series will drop this patch as per discussion
though, so the issue is moot.

>
>
> vim +577 mm/userfaultfd.c
>
>    508
>    509  static __always_inline ssize_t mfill_atomic(struct mm_struct *dst_mm,
>    510                                              unsigned long src_start,
>    511                                              const struct uffdio_range *dst,
>    512                                              atomic_t *mmap_changing,
>    513                                              uffd_flags_t flags)
>    514  {
>    515          struct vm_area_struct *dst_vma;
>    516          ssize_t err;
>    517          pmd_t *dst_pmd;
>    518          unsigned long src_addr, dst_addr;
>    519          long copied;
>    520          struct page *page;
>    521
>    522          /*
>    523           * Sanitize the command parameters:
>    524           */
>    525          BUG_ON(dst->start & ~PAGE_MASK);
>    526          BUG_ON(dst->len & ~PAGE_MASK);
>    527
>    528          /* Does the address range wrap, or is the span zero-sized? */
>    529          BUG_ON(src_start + dst->len <= src_start);
>    530          BUG_ON(dst->start + dst->len <= dst->start);
>    531
>    532          src_addr = src_start;
>    533          dst_addr = dst->start;
>    534          copied = 0;
>    535          page = NULL;
>    536  retry:
>    537          mmap_read_lock(dst_mm);
>    538
>    539          /*
>    540           * If memory mappings are changing because of non-cooperative
>    541           * operation (e.g. mremap) running in parallel, bail out and
>    542           * request the user to retry later
>    543           */
>    544          err = -EAGAIN;
>    545          if (mmap_changing && atomic_read(mmap_changing))
>    546                  goto out_unlock;
>    547
>    548          /*
>    549           * Make sure the vma is not shared, that the dst range is
>    550           * both valid and fully within a single existing vma.
>    551           */
>    552          err = -ENOENT;
>    553          dst_vma = find_dst_vma(dst_mm, dst);
>    554          if (!dst_vma)
>    555                  goto out_unlock;
>    556
>    557          err = -EINVAL;
>    558          /*
>    559           * shmem_zero_setup is invoked in mmap for MAP_ANONYMOUS|MAP_SHARED but
>    560           * it will overwrite vm_ops, so vma_is_anonymous must return false.
>    561           */
>    562          if (WARN_ON_ONCE(vma_is_anonymous(dst_vma) &&
>    563              dst_vma->vm_flags & VM_SHARED))
>    564                  goto out_unlock;
>    565
>    566          /*
>    567           * validate 'mode' now that we know the dst_vma: don't allow
>    568           * a wrprotect copy if the userfaultfd didn't register as WP.
>    569           */
>    570          if ((flags & MFILL_ATOMIC_WP) && !(dst_vma->vm_flags & VM_UFFD_WP))
>    571                  goto out_unlock;
>    572
>    573          /*
>    574           * If this is a HUGETLB vma, pass off to appropriate routine
>    575           */
>    576          if (is_vm_hugetlb_page(dst_vma))
>  > 577                  return  mfill_atomic_hugetlb(dst_vma, src_start, dst, flags);
>    578
>    579          if (!vma_is_anonymous(dst_vma) && !vma_is_shmem(dst_vma))
>    580                  goto out_unlock;
>    581          if (!vma_is_shmem(dst_vma) &&
>    582              (flags & MFILL_ATOMIC_MODE_MASK) == MFILL_ATOMIC_CONTINUE)
>    583                  goto out_unlock;
>    584
>    585          /*
>    586           * Ensure the dst_vma has a anon_vma or this page
>    587           * would get a NULL anon_vma when moved in the
>    588           * dst_vma.
>    589           */
>    590          err = -ENOMEM;
>    591          if (!(dst_vma->vm_flags & VM_SHARED) &&
>    592              unlikely(anon_vma_prepare(dst_vma)))
>    593                  goto out_unlock;
>    594
>    595          while (src_addr < src_start + dst->len) {
>    596                  pmd_t dst_pmdval;
>    597
>    598                  BUG_ON(dst_addr >= dst->start + dst->len);
>    599
>    600                  dst_pmd = mm_alloc_pmd(dst_mm, dst_addr);
>    601                  if (unlikely(!dst_pmd)) {
>    602                          err = -ENOMEM;
>    603                          break;
>    604                  }
>    605
>    606                  dst_pmdval = pmdp_get_lockless(dst_pmd);
>    607                  /*
>    608                   * If the dst_pmd is mapped as THP don't
>    609                   * override it and just be strict.
>    610                   */
>    611                  if (unlikely(pmd_trans_huge(dst_pmdval))) {
>    612                          err = -EEXIST;
>    613                          break;
>    614                  }
>    615                  if (unlikely(pmd_none(dst_pmdval)) &&
>    616                      unlikely(__pte_alloc(dst_mm, dst_pmd))) {
>    617                          err = -ENOMEM;
>    618                          break;
>    619                  }
>    620                  /* If an huge pmd materialized from under us fail */
>    621                  if (unlikely(pmd_trans_huge(*dst_pmd))) {
>    622                          err = -EFAULT;
>    623                          break;
>    624                  }
>    625
>    626                  BUG_ON(pmd_none(*dst_pmd));
>    627                  BUG_ON(pmd_trans_huge(*dst_pmd));
>    628
>    629                  err = mfill_atomic_pte(dst_pmd, dst_vma, dst_addr,
>    630                                         src_addr, &page, flags);
>    631                  cond_resched();
>    632
>    633                  if (unlikely(err == -ENOENT)) {
>    634                          void *page_kaddr;
>    635
>    636                          mmap_read_unlock(dst_mm);
>    637                          BUG_ON(!page);
>    638
>    639                          page_kaddr = kmap_local_page(page);
>    640                          err = copy_from_user(page_kaddr,
>    641                                               (const void __user *) src_addr,
>    642                                               PAGE_SIZE);
>    643                          kunmap_local(page_kaddr);
>    644                          if (unlikely(err)) {
>    645                                  err = -EFAULT;
>    646                                  goto out;
>    647                          }
>    648                          flush_dcache_page(page);
>    649                          goto retry;
>    650                  } else
>    651                          BUG_ON(page);
>    652
>    653                  if (!err) {
>    654                          dst_addr += PAGE_SIZE;
>    655                          src_addr += PAGE_SIZE;
>    656                          copied += PAGE_SIZE;
>    657
>    658                          if (fatal_signal_pending(current))
>    659                                  err = -EINTR;
>    660                  }
>    661                  if (err)
>    662                          break;
>    663          }
>    664
>    665  out_unlock:
>    666          mmap_read_unlock(dst_mm);
>    667  out:
>    668          if (page)
>    669                  put_page(page);
>    670          BUG_ON(copied < 0);
>    671          BUG_ON(err > 0);
>    672          BUG_ON(!copied && !err);
>    673          return copied ? copied : err;
>    674  }
>    675
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ