[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250702034720.53574-1-lizhe.67@bytedance.com>
Date: Wed, 2 Jul 2025 11:47:20 +0800
From: lizhe.67@...edance.com
To: dan.carpenter@...aro.org
Cc: alex.williamson@...hat.com,
david@...hat.com,
jgg@...pe.ca,
kvm@...r.kernel.org,
linux-kernel@...r.kernel.org,
lizhe.67@...edance.com,
lkp@...el.com,
oe-kbuild-all@...ts.linux.dev,
oe-kbuild@...ts.linux.dev,
peterx@...hat.com
Subject: Re: [PATCH 3/4] vfio/type1: introduce a new member has_rsvd for struct vfio_dma
On Tue, 1 Jul 2025 18:13:48 +0300, dan.carpenter@...aro.org wrote:
> New smatch warnings:
> drivers/vfio/vfio_iommu_type1.c:788 vfio_pin_pages_remote() error: uninitialized symbol 'rsvd'.
>
> Old smatch warnings:
> drivers/vfio/vfio_iommu_type1.c:2376 vfio_iommu_type1_attach_group() warn: '&group->next' not removed from list
>
> vim +/rsvd +788 drivers/vfio/vfio_iommu_type1.c
>
> 8f0d5bb95f763c Kirti Wankhede 2016-11-17 684 static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
> 0635559233434a Alex Williamson 2025-02-18 685 unsigned long npage, unsigned long *pfn_base,
> 4b6c33b3229678 Daniel Jordan 2021-02-19 686 unsigned long limit, struct vfio_batch *batch)
> 73fa0d10d077d9 Alex Williamson 2012-07-31 687 {
> 4d83de6da265cd Daniel Jordan 2021-02-19 688 unsigned long pfn;
> 4d83de6da265cd Daniel Jordan 2021-02-19 689 struct mm_struct *mm = current->mm;
> 6c38c055cc4c0a Alex Williamson 2016-12-30 690 long ret, pinned = 0, lock_acct = 0;
> 89c29def6b0101 Alex Williamson 2018-06-02 691 bool rsvd;
> a54eb55045ae9b Kirti Wankhede 2016-11-17 692 dma_addr_t iova = vaddr - dma->vaddr + dma->iova;
> 166fd7d94afdac Alex Williamson 2013-06-21 693
> 6c38c055cc4c0a Alex Williamson 2016-12-30 694 /* This code path is only user initiated */
> 4d83de6da265cd Daniel Jordan 2021-02-19 695 if (!mm)
> 166fd7d94afdac Alex Williamson 2013-06-21 696 return -ENODEV;
> 73fa0d10d077d9 Alex Williamson 2012-07-31 697
> 4d83de6da265cd Daniel Jordan 2021-02-19 698 if (batch->size) {
> 4d83de6da265cd Daniel Jordan 2021-02-19 699 /* Leftover pages in batch from an earlier call. */
> 4d83de6da265cd Daniel Jordan 2021-02-19 700 *pfn_base = page_to_pfn(batch->pages[batch->offset]);
> 4d83de6da265cd Daniel Jordan 2021-02-19 701 pfn = *pfn_base;
> 89c29def6b0101 Alex Williamson 2018-06-02 702 rsvd = is_invalid_reserved_pfn(*pfn_base);
When batch->size is not zero, we initialize rsvd here.
> 4d83de6da265cd Daniel Jordan 2021-02-19 703 } else {
> 4d83de6da265cd Daniel Jordan 2021-02-19 704 *pfn_base = 0;
When the value of batch->size is zero, we set the value of *pfn_base
to zero and do not initialize rsvd for the time being.
> 5c6c2b21ecc9ad Alex Williamson 2013-06-21 705 }
> 5c6c2b21ecc9ad Alex Williamson 2013-06-21 706
> eb996eec783c1e Alex Williamson 2025-02-18 707 if (unlikely(disable_hugepages))
> eb996eec783c1e Alex Williamson 2025-02-18 708 npage = 1;
> eb996eec783c1e Alex Williamson 2025-02-18 709
> 4d83de6da265cd Daniel Jordan 2021-02-19 710 while (npage) {
> 4d83de6da265cd Daniel Jordan 2021-02-19 711 if (!batch->size) {
> 4d83de6da265cd Daniel Jordan 2021-02-19 712 /* Empty batch, so refill it. */
> eb996eec783c1e Alex Williamson 2025-02-18 713 ret = vaddr_get_pfns(mm, vaddr, npage, dma->prot,
> eb996eec783c1e Alex Williamson 2025-02-18 714 &pfn, batch);
> be16c1fd99f41a Daniel Jordan 2021-02-19 715 if (ret < 0)
> 4d83de6da265cd Daniel Jordan 2021-02-19 716 goto unpin_out;
> 166fd7d94afdac Alex Williamson 2013-06-21 717
> 4d83de6da265cd Daniel Jordan 2021-02-19 718 if (!*pfn_base) {
> 4d83de6da265cd Daniel Jordan 2021-02-19 719 *pfn_base = pfn;
> 4d83de6da265cd Daniel Jordan 2021-02-19 720 rsvd = is_invalid_reserved_pfn(*pfn_base);
Therefore, for the first loop, when batch->size is zero, *pfn_base must
be zero, which will then lead to the initialization of rsvd.
> 4d83de6da265cd Daniel Jordan 2021-02-19 721 }
>
> If "*pfn_base" is true then "rsvd" is uninitialized.
>
> eb996eec783c1e Alex Williamson 2025-02-18 722
> eb996eec783c1e Alex Williamson 2025-02-18 723 /* Handle pfnmap */
> eb996eec783c1e Alex Williamson 2025-02-18 724 if (!batch->size) {
> eb996eec783c1e Alex Williamson 2025-02-18 725 if (pfn != *pfn_base + pinned || !rsvd)
> eb996eec783c1e Alex Williamson 2025-02-18 726 goto out;
>
> goto out;
>
> eb996eec783c1e Alex Williamson 2025-02-18 727
> eb996eec783c1e Alex Williamson 2025-02-18 728 pinned += ret;
> eb996eec783c1e Alex Williamson 2025-02-18 729 npage -= ret;
> eb996eec783c1e Alex Williamson 2025-02-18 730 vaddr += (PAGE_SIZE * ret);
> eb996eec783c1e Alex Williamson 2025-02-18 731 iova += (PAGE_SIZE * ret);
> eb996eec783c1e Alex Williamson 2025-02-18 732 continue;
> eb996eec783c1e Alex Williamson 2025-02-18 733 }
> 166fd7d94afdac Alex Williamson 2013-06-21 734 }
> 166fd7d94afdac Alex Williamson 2013-06-21 735
> 4d83de6da265cd Daniel Jordan 2021-02-19 736 /*
> eb996eec783c1e Alex Williamson 2025-02-18 737 * pfn is preset for the first iteration of this inner loop
> eb996eec783c1e Alex Williamson 2025-02-18 738 * due to the fact that vaddr_get_pfns() needs to provide the
> eb996eec783c1e Alex Williamson 2025-02-18 739 * initial pfn for pfnmaps. Therefore to reduce redundancy,
> eb996eec783c1e Alex Williamson 2025-02-18 740 * the next pfn is fetched at the end of the loop.
> eb996eec783c1e Alex Williamson 2025-02-18 741 * A PageReserved() page could still qualify as page backed
> eb996eec783c1e Alex Williamson 2025-02-18 742 * and rsvd here, and therefore continues to use the batch.
> 4d83de6da265cd Daniel Jordan 2021-02-19 743 */
> 4d83de6da265cd Daniel Jordan 2021-02-19 744 while (true) {
> 6a2d9b72168041 Li Zhe 2025-06-30 745 long nr_pages, acct_pages = 0;
> 6a2d9b72168041 Li Zhe 2025-06-30 746
> 4d83de6da265cd Daniel Jordan 2021-02-19 747 if (pfn != *pfn_base + pinned ||
> 4d83de6da265cd Daniel Jordan 2021-02-19 748 rsvd != is_invalid_reserved_pfn(pfn))
> 4d83de6da265cd Daniel Jordan 2021-02-19 749 goto out;
> 4d83de6da265cd Daniel Jordan 2021-02-19 750
> 6a2d9b72168041 Li Zhe 2025-06-30 751 nr_pages = contig_pages(dma, batch, iova);
> 6a2d9b72168041 Li Zhe 2025-06-30 752 if (!rsvd) {
> 6a2d9b72168041 Li Zhe 2025-06-30 753 acct_pages = nr_pages;
> 6a2d9b72168041 Li Zhe 2025-06-30 754 acct_pages -= vpfn_pages(dma, iova, nr_pages);
> 6a2d9b72168041 Li Zhe 2025-06-30 755 }
> 6a2d9b72168041 Li Zhe 2025-06-30 756
> 4d83de6da265cd Daniel Jordan 2021-02-19 757 /*
> 4d83de6da265cd Daniel Jordan 2021-02-19 758 * Reserved pages aren't counted against the user,
> 4d83de6da265cd Daniel Jordan 2021-02-19 759 * externally pinned pages are already counted against
> 4d83de6da265cd Daniel Jordan 2021-02-19 760 * the user.
> 4d83de6da265cd Daniel Jordan 2021-02-19 761 */
> 6a2d9b72168041 Li Zhe 2025-06-30 762 if (acct_pages) {
> 48d8476b41eed6 Alex Williamson 2018-05-11 763 if (!dma->lock_cap &&
> 6a2d9b72168041 Li Zhe 2025-06-30 764 mm->locked_vm + lock_acct + acct_pages > limit) {
> 6c38c055cc4c0a Alex Williamson 2016-12-30 765 pr_warn("%s: RLIMIT_MEMLOCK (%ld) exceeded\n",
> 6c38c055cc4c0a Alex Williamson 2016-12-30 766 __func__, limit << PAGE_SHIFT);
> 0cfef2b7410b64 Alex Williamson 2017-04-13 767 ret = -ENOMEM;
> 0cfef2b7410b64 Alex Williamson 2017-04-13 768 goto unpin_out;
> 166fd7d94afdac Alex Williamson 2013-06-21 769 }
> 6a2d9b72168041 Li Zhe 2025-06-30 770 lock_acct += acct_pages;
> a54eb55045ae9b Kirti Wankhede 2016-11-17 771 }
> 4d83de6da265cd Daniel Jordan 2021-02-19 772
> 6a2d9b72168041 Li Zhe 2025-06-30 773 pinned += nr_pages;
> 6a2d9b72168041 Li Zhe 2025-06-30 774 npage -= nr_pages;
> 6a2d9b72168041 Li Zhe 2025-06-30 775 vaddr += PAGE_SIZE * nr_pages;
> 6a2d9b72168041 Li Zhe 2025-06-30 776 iova += PAGE_SIZE * nr_pages;
> 6a2d9b72168041 Li Zhe 2025-06-30 777 batch->offset += nr_pages;
> 6a2d9b72168041 Li Zhe 2025-06-30 778 batch->size -= nr_pages;
> 4d83de6da265cd Daniel Jordan 2021-02-19 779
> 4d83de6da265cd Daniel Jordan 2021-02-19 780 if (!batch->size)
> 4d83de6da265cd Daniel Jordan 2021-02-19 781 break;
> 4d83de6da265cd Daniel Jordan 2021-02-19 782
> 4d83de6da265cd Daniel Jordan 2021-02-19 783 pfn = page_to_pfn(batch->pages[batch->offset]);
> 4d83de6da265cd Daniel Jordan 2021-02-19 784 }
> a54eb55045ae9b Kirti Wankhede 2016-11-17 785 }
> 166fd7d94afdac Alex Williamson 2013-06-21 786
> 6c38c055cc4c0a Alex Williamson 2016-12-30 787 out:
> 20448310d6b71d Li Zhe 2025-06-30 @788 dma->has_rsvd |= rsvd;
> ^^^^
In summary, it is likely to be a false alarm.
Please correct me if I am wrong.
Thanks,
Zhe
Powered by blists - more mailing lists