Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <202201170247.Cir3moOM-lkp@intel.com>
In-Reply-To: <202201170247.Cir3moOM-lkp@intel.com>
From:   Axel Rasmussen <axelrasmussen@google.com>
Date:   Wed, 19 Jan 2022 12:38:48 -0800
Message-ID: <CAJHvVchPpphLefKEOy_C6+0xrGtA3mNCBzQZ3j2i2RgXX3arXQ@mail.gmail.com>
Subject: Re: [ammarfaizi2-block:google/android/kernel/common/android12-5.4
 6517/9999] fs/userfaultfd.c:1519:9: warning: variable 'ioctls_out' set but
 not used
To:     kernel test robot <lkp@intel.com>
Cc:     llvm@lists.linux.dev, kbuild-all@lists.01.org,
        "GNU/Weeb Mailing List" <gwml@gnuweeb.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Todd Kjos <tkjos@google.com>, Peter Xu <peterx@redhat.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Linux Memory Management List <linux-mm@kvack.org>,
        Lokesh Gidra <lokeshgidra@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Lokesh, I only spent a few moments looking, but my suspicion is that
this is the result of an incomplete backport of these patches to the
Android kernel?

The UFFDIO_CONTINUE patches introduced ioctls_out since the
computation of it got more complicated, and those same patches also
modified the put_user() on line 1533 here to use ioctls_out. I think
Android backported the first part, but left out the second?

On Sun, Jan 16, 2022 at 10:11 AM kernel test robot <lkp@intel.com> wrote:
>
> Hi Axel,
>
> FYI, the error/warning still remains.
>
> tree:   https://github.com/ammarfaizi2/linux-block google/android/kernel/common/android12-5.4
> head:   bdf17ba628090156b539b1474eb5c636eeaf571b
> commit: b69f713e60d03ae448e5c9fd92e5b0b193dea7be [6517/9999] BACKPORT: FROMGIT: userfaultfd: add UFFDIO_CONTINUE ioctl
> config: x86_64-randconfig-a012 (https://download.01.org/0day-ci/archive/20220117/202201170247.Cir3moOM-lkp@intel.com/config)
> compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project c63a3175c2947e8c1a2d3bbe16a8586600705c54)
> 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/ammarfaizi2/linux-block/commit/b69f713e60d03ae448e5c9fd92e5b0b193dea7be
>         git remote add ammarfaizi2-block https://github.com/ammarfaizi2/linux-block
>         git fetch --no-tags ammarfaizi2-block google/android/kernel/common/android12-5.4
>         git checkout b69f713e60d03ae448e5c9fd92e5b0b193dea7be
>         # save the config file to linux build tree
>         mkdir build_dir
>         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 as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> >> fs/userfaultfd.c:1519:9: warning: variable 'ioctls_out' set but not used [-Wunused-but-set-variable]
>                    __u64 ioctls_out;
>                          ^
>    1 warning generated.
>
>
> vim +/ioctls_out +1519 fs/userfaultfd.c
>
>   1311
>   1312  static int userfaultfd_register(struct userfaultfd_ctx *ctx,
>   1313                                  unsigned long arg)
>   1314  {
>   1315          struct mm_struct *mm = ctx->mm;
>   1316          struct vm_area_struct *vma, *prev, *cur;
>   1317          int ret;
>   1318          struct uffdio_register uffdio_register;
>   1319          struct uffdio_register __user *user_uffdio_register;
>   1320          unsigned long vm_flags, new_flags;
>   1321          bool found;
>   1322          bool basic_ioctls;
>   1323          unsigned long start, end, vma_end;
>   1324
>   1325          user_uffdio_register = (struct uffdio_register __user *) arg;
>   1326
>   1327          ret = -EFAULT;
>   1328          if (copy_from_user(&uffdio_register, user_uffdio_register,
>   1329                             sizeof(uffdio_register)-sizeof(__u64)))
>   1330                  goto out;
>   1331
>   1332          ret = -EINVAL;
>   1333          if (!uffdio_register.mode)
>   1334                  goto out;
>   1335          if (uffdio_register.mode & ~UFFD_API_REGISTER_MODES)
>   1336                  goto out;
>   1337          vm_flags = 0;
>   1338          if (uffdio_register.mode & UFFDIO_REGISTER_MODE_MISSING)
>   1339                  vm_flags |= VM_UFFD_MISSING;
>   1340          if (uffdio_register.mode & UFFDIO_REGISTER_MODE_WP) {
>   1341                  vm_flags |= VM_UFFD_WP;
>   1342                  /*
>   1343                   * FIXME: remove the below error constraint by
>   1344                   * implementing the wprotect tracking mode.
>   1345                   */
>   1346                  ret = -EINVAL;
>   1347                  goto out;
>   1348          }
>   1349          if (uffdio_register.mode & UFFDIO_REGISTER_MODE_MINOR) {
>   1350  #ifndef CONFIG_HAVE_ARCH_USERFAULTFD_MINOR
>   1351                  goto out;
>   1352  #endif
>   1353                  vm_flags |= VM_UFFD_MINOR;
>   1354          }
>   1355
>   1356          ret = validate_range(mm, &uffdio_register.range.start,
>   1357                               uffdio_register.range.len);
>   1358          if (ret)
>   1359                  goto out;
>   1360
>   1361          start = uffdio_register.range.start;
>   1362          end = start + uffdio_register.range.len;
>   1363
>   1364          ret = -ENOMEM;
>   1365          if (!mmget_not_zero(mm))
>   1366                  goto out;
>   1367
>   1368          down_write(&mm->mmap_sem);
>   1369          if (!mmget_still_valid(mm))
>   1370                  goto out_unlock;
>   1371          vma = find_vma_prev(mm, start, &prev);
>   1372          if (!vma)
>   1373                  goto out_unlock;
>   1374
>   1375          /* check that there's at least one vma in the range */
>   1376          ret = -EINVAL;
>   1377          if (vma->vm_start >= end)
>   1378                  goto out_unlock;
>   1379
>   1380          /*
>   1381           * If the first vma contains huge pages, make sure start address
>   1382           * is aligned to huge page size.
>   1383           */
>   1384          if (is_vm_hugetlb_page(vma)) {
>   1385                  unsigned long vma_hpagesize = vma_kernel_pagesize(vma);
>   1386
>   1387                  if (start & (vma_hpagesize - 1))
>   1388                          goto out_unlock;
>   1389          }
>   1390
>   1391          /*
>   1392           * Search for not compatible vmas.
>   1393           */
>   1394          found = false;
>   1395          basic_ioctls = false;
>   1396          for (cur = vma; cur && cur->vm_start < end; cur = cur->vm_next) {
>   1397                  cond_resched();
>   1398
>   1399                  BUG_ON(!!cur->vm_userfaultfd_ctx.ctx ^
>   1400                         !!(cur->vm_flags & __VM_UFFD_FLAGS));
>   1401
>   1402                  /* check not compatible vmas */
>   1403                  ret = -EINVAL;
>   1404                  if (!vma_can_userfault(cur, vm_flags))
>   1405                          goto out_unlock;
>   1406
>   1407                  /*
>   1408                   * UFFDIO_COPY will fill file holes even without
>   1409                   * PROT_WRITE. This check enforces that if this is a
>   1410                   * MAP_SHARED, the process has write permission to the backing
>   1411                   * file. If VM_MAYWRITE is set it also enforces that on a
>   1412                   * MAP_SHARED vma: there is no F_WRITE_SEAL and no further
>   1413                   * F_WRITE_SEAL can be taken until the vma is destroyed.
>   1414                   */
>   1415                  ret = -EPERM;
>   1416                  if (unlikely(!(cur->vm_flags & VM_MAYWRITE)))
>   1417                          goto out_unlock;
>   1418
>   1419                  /*
>   1420                   * If this vma contains ending address, and huge pages
>   1421                   * check alignment.
>   1422                   */
>   1423                  if (is_vm_hugetlb_page(cur) && end <= cur->vm_end &&
>   1424                      end > cur->vm_start) {
>   1425                          unsigned long vma_hpagesize = vma_kernel_pagesize(cur);
>   1426
>   1427                          ret = -EINVAL;
>   1428
>   1429                          if (end & (vma_hpagesize - 1))
>   1430                                  goto out_unlock;
>   1431                  }
>   1432
>   1433                  /*
>   1434                   * Check that this vma isn't already owned by a
>   1435                   * different userfaultfd. We can't allow more than one
>   1436                   * userfaultfd to own a single vma simultaneously or we
>   1437                   * wouldn't know which one to deliver the userfaults to.
>   1438                   */
>   1439                  ret = -EBUSY;
>   1440                  if (cur->vm_userfaultfd_ctx.ctx &&
>   1441                      cur->vm_userfaultfd_ctx.ctx != ctx)
>   1442                          goto out_unlock;
>   1443
>   1444                  /*
>   1445                   * Note vmas containing huge pages
>   1446                   */
>   1447                  if (is_vm_hugetlb_page(cur))
>   1448                          basic_ioctls = true;
>   1449
>   1450                  found = true;
>   1451          }
>   1452          BUG_ON(!found);
>   1453
>   1454          if (vma->vm_start < start)
>   1455                  prev = vma;
>   1456
>   1457          ret = 0;
>   1458          do {
>   1459                  cond_resched();
>   1460
>   1461                  BUG_ON(!vma_can_userfault(vma, vm_flags));
>   1462                  BUG_ON(vma->vm_userfaultfd_ctx.ctx &&
>   1463                         vma->vm_userfaultfd_ctx.ctx != ctx);
>   1464                  WARN_ON(!(vma->vm_flags & VM_MAYWRITE));
>   1465
>   1466                  /*
>   1467                   * Nothing to do: this vma is already registered into this
>   1468                   * userfaultfd and with the right tracking mode too.
>   1469                   */
>   1470                  if (vma->vm_userfaultfd_ctx.ctx == ctx &&
>   1471                      (vma->vm_flags & vm_flags) == vm_flags)
>   1472                          goto skip;
>   1473
>   1474                  if (vma->vm_start > start)
>   1475                          start = vma->vm_start;
>   1476                  vma_end = min(end, vma->vm_end);
>   1477
>   1478                  new_flags = (vma->vm_flags & ~__VM_UFFD_FLAGS) | vm_flags;
>   1479                  prev = vma_merge(mm, prev, start, vma_end, new_flags,
>   1480                                   vma->anon_vma, vma->vm_file, vma->vm_pgoff,
>   1481                                   vma_policy(vma),
>   1482                                   ((struct vm_userfaultfd_ctx){ ctx }),
>   1483                                   vma_get_anon_name(vma));
>   1484                  if (prev) {
>   1485                          vma = prev;
>   1486                          goto next;
>   1487                  }
>   1488                  if (vma->vm_start < start) {
>   1489                          ret = split_vma(mm, vma, start, 1);
>   1490                          if (ret)
>   1491                                  break;
>   1492                  }
>   1493                  if (vma->vm_end > end) {
>   1494                          ret = split_vma(mm, vma, end, 0);
>   1495                          if (ret)
>   1496                                  break;
>   1497                  }
>   1498          next:
>   1499                  /*
>   1500                   * In the vma_merge() successful mprotect-like case 8:
>   1501                   * the next vma was merged into the current one and
>   1502                   * the current one has not been updated yet.
>   1503                   */
>   1504                  vma->vm_flags = new_flags;
>   1505                  vma->vm_userfaultfd_ctx.ctx = ctx;
>   1506
>   1507                  if (is_vm_hugetlb_page(vma) && uffd_disable_huge_pmd_share(vma))
>   1508                          hugetlb_unshare_all_pmds(vma);
>   1509
>   1510          skip:
>   1511                  prev = vma;
>   1512                  start = vma->vm_end;
>   1513                  vma = vma->vm_next;
>   1514          } while (vma && vma->vm_start < end);
>   1515  out_unlock:
>   1516          up_write(&mm->mmap_sem);
>   1517          mmput(mm);
>   1518          if (!ret) {
> > 1519                  __u64 ioctls_out;
>   1520
>   1521                  ioctls_out = basic_ioctls ? UFFD_API_RANGE_IOCTLS_BASIC :
>   1522                      UFFD_API_RANGE_IOCTLS;
>   1523
>   1524                  /* CONTINUE ioctl is only supported for MINOR ranges. */
>   1525                  if (!(uffdio_register.mode & UFFDIO_REGISTER_MODE_MINOR))
>   1526                          ioctls_out &= ~((__u64)1 << _UFFDIO_CONTINUE);
>   1527
>   1528                  /*
>   1529                   * Now that we scanned all vmas we can already tell
>   1530                   * userland which ioctls methods are guaranteed to
>   1531                   * succeed on this range.
>   1532                   */
>   1533                  if (put_user(basic_ioctls ? UFFD_API_RANGE_IOCTLS_BASIC :
>   1534                               UFFD_API_RANGE_IOCTLS,
>   1535                               &user_uffdio_register->ioctls))
>   1536                          ret = -EFAULT;
>   1537          }
>   1538  out:
>   1539          return ret;
>   1540  }
>   1541
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org