Re: [PATCH] virtio_mem: prevent overflow with subblock size

kernel test robot
2020-06-09T00:16:52Z

[-- Attachment #1: Type: text/plain, Size: 9633 bytes --]

Hi "Michael,

I love your patch! Perhaps something to improve:

[auto build test WARNING on vhost/vhost]
[also build test WARNING on next-20200608]
[cannot apply to linus/master linux/master v5.7]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Michael-S-Tsirkin/virtio_mem-prevent-overflow-with-subblock-size/20200608-141805
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project e429cffd4f228f70c1d9df0e5d77c08590dd9766)
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 x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64

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 >>, old ones prefixed by <<):

>> drivers/virtio/virtio_mem.c:1721:4: warning: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long long') [-Wformat]
vm->subblock_size);
^~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:33: note: expanded from macro 'dev_info'
_dev_info(dev, dev_fmt(fmt), ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
1 warning generated.

vim +1721 drivers/virtio/virtio_mem.c

5f1f79bbc9e26f David Hildenbrand 2020-05-07 1642
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1643 static int virtio_mem_init(struct virtio_mem *vm)
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1644 {
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1645 const uint64_t phys_limit = 1UL << MAX_PHYSMEM_BITS;
f2af6d3978d74a David Hildenbrand 2020-05-07 1646 uint16_t node_id;
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1647
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1648 if (!vm->vdev->config->get) {
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1649 dev_err(&vm->vdev->dev, "config access disabled\n");
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1650 return -EINVAL;
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1651 }
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1652
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1653 /*
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1654 * We don't want to (un)plug or reuse any memory when in kdump. The
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1655 * memory is still accessible (but not mapped).
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1656 */
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1657 if (is_kdump_kernel()) {
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1658 dev_warn(&vm->vdev->dev, "disabled in kdump kernel\n");
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1659 return -EBUSY;
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1660 }
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1661
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1662 /* Fetch all properties that can't change. */
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1663 virtio_cread(vm->vdev, struct virtio_mem_config, plugged_size,
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1664 &vm->plugged_size);
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1665 virtio_cread(vm->vdev, struct virtio_mem_config, block_size,
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1666 &vm->device_block_size);
f2af6d3978d74a David Hildenbrand 2020-05-07 1667 virtio_cread(vm->vdev, struct virtio_mem_config, node_id,
f2af6d3978d74a David Hildenbrand 2020-05-07 1668 &node_id);
f2af6d3978d74a David Hildenbrand 2020-05-07 1669 vm->nid = virtio_mem_translate_node_id(vm, node_id);
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1670 virtio_cread(vm->vdev, struct virtio_mem_config, addr, &vm->addr);
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1671 virtio_cread(vm->vdev, struct virtio_mem_config, region_size,
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1672 &vm->region_size);
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1673
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1674 /*
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1675 * We always hotplug memory in memory block granularity. This way,
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1676 * we have to wait for exactly one memory block to online.
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1677 */
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1678 if (vm->device_block_size > memory_block_size_bytes()) {
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1679 dev_err(&vm->vdev->dev,
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1680 "The block size is not supported (too big).\n");
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1681 return -EINVAL;
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1682 }
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1683
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1684 /* bad device setup - warn only */
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1685 if (!IS_ALIGNED(vm->addr, memory_block_size_bytes()))
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1686 dev_warn(&vm->vdev->dev,
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1687 "The alignment of the physical start address can make some memory unusable.\n");
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1688 if (!IS_ALIGNED(vm->addr + vm->region_size, memory_block_size_bytes()))
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1689 dev_warn(&vm->vdev->dev,
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1690 "The alignment of the physical end address can make some memory unusable.\n");
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1691 if (vm->addr + vm->region_size > phys_limit)
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1692 dev_warn(&vm->vdev->dev,
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1693 "Some memory is not addressable. This can make some memory unusable.\n");
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1694
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1695 /*
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1696 * Calculate the subblock size:
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1697 * - At least MAX_ORDER - 1 / pageblock_order.
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1698 * - At least the device block size.
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1699 * In the worst case, a single subblock per memory block.
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1700 */
09760a643f90c4 Michael S. Tsirkin 2020-06-08 1701 vm->subblock_size = PAGE_SIZE * 1ul << max_t(uint32_t, MAX_ORDER - 1,
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1702 pageblock_order);
09760a643f90c4 Michael S. Tsirkin 2020-06-08 1703 vm->subblock_size = max_t(uint64_t, vm->device_block_size,
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1704 vm->subblock_size);
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1705 vm->nb_sb_per_mb = memory_block_size_bytes() / vm->subblock_size;
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1706
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1707 /* Round up to the next full memory block */
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1708 vm->first_mb_id = virtio_mem_phys_to_mb_id(vm->addr - 1 +
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1709 memory_block_size_bytes());
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1710 vm->next_mb_id = vm->first_mb_id;
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1711 vm->last_mb_id = virtio_mem_phys_to_mb_id(vm->addr +
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1712 vm->region_size) - 1;
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1713
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1714 dev_info(&vm->vdev->dev, "start address: 0x%llx", vm->addr);
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1715 dev_info(&vm->vdev->dev, "region size: 0x%llx", vm->region_size);
09760a643f90c4 Michael S. Tsirkin 2020-06-08 1716 dev_info(&vm->vdev->dev, "device block size: 0x%llx",
09760a643f90c4 Michael S. Tsirkin 2020-06-08 1717 (unsigned long long)vm->device_block_size);
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1718 dev_info(&vm->vdev->dev, "memory block size: 0x%lx",
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1719 memory_block_size_bytes());
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1720 dev_info(&vm->vdev->dev, "subblock size: 0x%x",
5f1f79bbc9e26f David Hildenbrand 2020-05-07 @1721 vm->subblock_size);
f2af6d3978d74a David Hildenbrand 2020-05-07 1722 if (vm->nid != NUMA_NO_NODE)
f2af6d3978d74a David Hildenbrand 2020-05-07 1723 dev_info(&vm->vdev->dev, "nid: %d", vm->nid);
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1724
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1725 return 0;
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1726 }
5f1f79bbc9e26f David Hildenbrand 2020-05-07 1727

:::::: The code at line 1721 was first introduced by commit
:::::: 5f1f79bbc9e26fa9412fa9522f957bb8f030c442 virtio-mem: Paravirtualized memory hotplug

:::::: TO: David Hildenbrand <david@redhat.com>
:::::: CC: Michael S. Tsirkin <mst@redhat.com>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 73449 bytes --]