Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753455AbcLFSWJ (ORCPT ); Tue, 6 Dec 2016 13:22:09 -0500 Received: from hqemgate16.nvidia.com ([216.228.121.65]:12796 "EHLO hqemgate16.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751654AbcLFSWH (ORCPT ); Tue, 6 Dec 2016 13:22:07 -0500 X-PGP-Universal: processed; by hqnvupgp07.nvidia.com on Mon, 05 Dec 2016 22:19:59 -0800 Subject: Re: [PATCH v2 1/2] vfio iommu type1: Fix size argument to vfio_find_dma() during DMA UNMAP. To: Alex Williamson References: <1481044410-23862-1-git-send-email-kwankhede@nvidia.com> <20161206103859.2fd91db0@t450s.home> CC: , , , , , X-Nvconfidentiality: public From: Kirti Wankhede Message-ID: Date: Tue, 6 Dec 2016 23:50:59 +0530 MIME-Version: 1.0 In-Reply-To: <20161206103859.2fd91db0@t450s.home> X-Originating-IP: [10.24.70.163] X-ClientProxiedBy: DRHKMAIL103.nvidia.com (10.25.59.17) To bgmail102.nvidia.com (10.25.59.11) Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2437 Lines: 60 On 12/6/2016 11:08 PM, Alex Williamson wrote: > On Tue, 6 Dec 2016 22:43:30 +0530 > Kirti Wankhede wrote: > >> vfio_dma keeps track of address range from (dma->iova + 0) to >> (dma->iova + dma->size - 1), while vfio_find_dma() search logic looks for >> range from (dma->iova + 1) to (dma->iova + dma->size). > > This is not true. The issue is with the non-inclusive address at the > end of a range being incompatible with passing zero for the range > size. Passing zero for the range size is a bit of a hack and doing > such triggers a corner case in the end of range detection where we test > (start + size <= dma->iova). Using <= here covers the non-inclusive > range end, ie. if the range was start=0x0, size=0x1000, the range is > actually 0x0-0xfff. Thus if start+size is 0x1000, it should not be > considered part of a range beginning at 0x1000. So there's no > incompatibility as implied in the statement above, it's more that using > zero for the size isn't compatible with matching the start address of > an existing vfio_dma. Thanks, > Makes sense. Updating the description of both patch. Thanks, Kirti > Alex > >> In vfio_find_dma(), when the start address is equal to dma->iova and size >> is 0, check for the end of search range makes it to take wrong side of >> RB-tree. That fails the search even though the address is present in >> mapped dma ranges. Due to this, in vfio_dma_do_unmap(), while checking >> boundary conditions, size should be set to 1 for verifying start address >> of unmap range. >> vfio_find_dma() is also used to verify last address in unmap range with >> size = 0, but in that case address to be searched is calculated with >> size - 1 and so it works correctly. >> >> Signed-off-by: Kirti Wankhede >> Signed-off-by: Neo Jia >> --- >> drivers/vfio/vfio_iommu_type1.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c >> index a28fbddb505c..8e9e94ccb2ff 100644 >> --- a/drivers/vfio/vfio_iommu_type1.c >> +++ b/drivers/vfio/vfio_iommu_type1.c >> @@ -826,7 +826,7 @@ again: >> * mappings within the range. >> */ >> if (iommu->v2) { >> - dma = vfio_find_dma(iommu, unmap->iova, 0); >> + dma = vfio_find_dma(iommu, unmap->iova, 1); >> if (dma && dma->iova != unmap->iova) { >> ret = -EINVAL; >> goto unlock; >