* [PATCH 0/2] ACPI/IORT: handle potential overflows @ 2018-12-18 18:48 Andrew Jones [not found] ` <20181218184841.20034-1-drjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Andrew Jones @ 2018-12-18 18:48 UTC (permalink / raw) To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-acpi-u79uwXL29TY76Z2rM5mHXA Cc: sudeep.holla-5wv7dgnIgG8, robin.murphy-5wv7dgnIgG8 I saw a couple places where overflows could potentially occur. I've only compile tested these changes. Thanks, drew Andrew Jones (2): ACPI/IORT: Handle potential overflow in iort_dma_setup iommu/dma: Handle potential overflow in iommu_dma_init_domain drivers/acpi/arm64/iort.c | 7 ++++++- drivers/iommu/dma-iommu.c | 15 +++++++++------ 2 files changed, 15 insertions(+), 7 deletions(-) -- 2.17.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <20181218184841.20034-1-drjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* [PATCH 1/2] ACPI/IORT: Handle potential overflow in iort_dma_setup [not found] ` <20181218184841.20034-1-drjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2018-12-18 18:48 ` Andrew Jones [not found] ` <20181218184841.20034-2-drjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2018-12-18 18:48 ` [PATCH 2/2] iommu/dma: Handle potential overflow in iommu_dma_init_domain Andrew Jones 1 sibling, 1 reply; 9+ messages in thread From: Andrew Jones @ 2018-12-18 18:48 UTC (permalink / raw) To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-acpi-u79uwXL29TY76Z2rM5mHXA Cc: sudeep.holla-5wv7dgnIgG8, robin.murphy-5wv7dgnIgG8 The sum of dmaaddr and size may overflow, particularly considering there are cases where size will be U64_MAX. Signed-off-by: Andrew Jones <drjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> --- drivers/acpi/arm64/iort.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c index 70f4e80b9246..a0f4c157ba5e 100644 --- a/drivers/acpi/arm64/iort.c +++ b/drivers/acpi/arm64/iort.c @@ -1002,7 +1002,12 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size) } if (!ret) { - msb = fls64(dmaaddr + size - 1); + u64 dmaaddr_max = dmaaddr + size - 1; + if (dmaaddr_max >= dmaaddr) + msb = fls64(dmaaddr_max); + else + msb = 64; + /* * Round-up to the power-of-two mask or set * the mask to the whole 64-bit address space -- 2.17.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
[parent not found: <20181218184841.20034-2-drjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH 1/2] ACPI/IORT: Handle potential overflow in iort_dma_setup [not found] ` <20181218184841.20034-2-drjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2018-12-19 12:21 ` Robin Murphy [not found] ` <1503e3b8-1a6c-3b66-fa1e-d13f4e19f31f-5wv7dgnIgG8@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Robin Murphy @ 2018-12-19 12:21 UTC (permalink / raw) To: Andrew Jones, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-acpi-u79uwXL29TY76Z2rM5mHXA Cc: sudeep.holla-5wv7dgnIgG8 On 18/12/2018 18:48, Andrew Jones wrote: > The sum of dmaaddr and size may overflow, particularly considering > there are cases where size will be U64_MAX. Only if the firmware is broken in the first place, though. It would be weird to describe an explicit _DMA range of base=0 and size=U64_MAX, because it's effectively the same as just not having one at all, but it's not strictly illegal. However, since the ACPI System Memory address space is at most 64-bit, anything that would actually overflow here is already describing an impossibility - really, we should probably scream even louder about a firmware bug and reject it entirely, rather than quietly hiding it. Robin. > Signed-off-by: Andrew Jones <drjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > --- > drivers/acpi/arm64/iort.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c > index 70f4e80b9246..a0f4c157ba5e 100644 > --- a/drivers/acpi/arm64/iort.c > +++ b/drivers/acpi/arm64/iort.c > @@ -1002,7 +1002,12 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size) > } > > if (!ret) { > - msb = fls64(dmaaddr + size - 1); > + u64 dmaaddr_max = dmaaddr + size - 1; > + if (dmaaddr_max >= dmaaddr) > + msb = fls64(dmaaddr_max); > + else > + msb = 64; > + > /* > * Round-up to the power-of-two mask or set > * the mask to the whole 64-bit address space > ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <1503e3b8-1a6c-3b66-fa1e-d13f4e19f31f-5wv7dgnIgG8@public.gmane.org>]
* Re: [PATCH 1/2] ACPI/IORT: Handle potential overflow in iort_dma_setup [not found] ` <1503e3b8-1a6c-3b66-fa1e-d13f4e19f31f-5wv7dgnIgG8@public.gmane.org> @ 2018-12-19 13:18 ` Andrew Jones [not found] ` <20181219131849.hziujd5zgclangce-gVz1Vyx/EOXkZJWtSm8s3NvLeJWuRmrY@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Andrew Jones @ 2018-12-19 13:18 UTC (permalink / raw) To: Robin Murphy Cc: linux-acpi-u79uwXL29TY76Z2rM5mHXA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, sudeep.holla-5wv7dgnIgG8 On Wed, Dec 19, 2018 at 12:21:35PM +0000, Robin Murphy wrote: > On 18/12/2018 18:48, Andrew Jones wrote: > > The sum of dmaaddr and size may overflow, particularly considering > > there are cases where size will be U64_MAX. > > Only if the firmware is broken in the first place, though. It would be weird > to describe an explicit _DMA range of base=0 and size=U64_MAX, because it's > effectively the same as just not having one at all, but it's not strictly > illegal. However, since the ACPI System Memory address space is at most > 64-bit, anything that would actually overflow here is already describing an > impossibility - really, we should probably scream even louder about a > firmware bug and reject it entirely, rather than quietly hiding it. Ah, looking again I see the paths. Either acpi_dma_get_range() returns success, in which case base and size are fine, or it returns an EINVAL, in which case base=size=0, or it returns ENODEV in which case base is zero, so size may be set to U64_MAX by rc_dma_get_range() with no problem. The !dev_is_pci(dev) path is also fine since base=0. While I agree that we should complain when firmware provides bad ACPI tables, my understanding of setting iort.memory_address_limit=64 was that it simply meant "no limit", regardless of the base. However I now see that it won't be used unless base=0. So I don't think we have a problem, and we don't even seem to be missing firmware sanity checks. It might be nice to have a comment explaining this stuff somewhere, but otherwise sorry for the noise. Thanks, drew > > Robin. > > > Signed-off-by: Andrew Jones <drjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > > --- > > drivers/acpi/arm64/iort.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c > > index 70f4e80b9246..a0f4c157ba5e 100644 > > --- a/drivers/acpi/arm64/iort.c > > +++ b/drivers/acpi/arm64/iort.c > > @@ -1002,7 +1002,12 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size) > > } > > if (!ret) { > > - msb = fls64(dmaaddr + size - 1); > > + u64 dmaaddr_max = dmaaddr + size - 1; > > + if (dmaaddr_max >= dmaaddr) > > + msb = fls64(dmaaddr_max); > > + else > > + msb = 64; > > + > > /* > > * Round-up to the power-of-two mask or set > > * the mask to the whole 64-bit address space > > ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <20181219131849.hziujd5zgclangce-gVz1Vyx/EOXkZJWtSm8s3NvLeJWuRmrY@public.gmane.org>]
* Re: [PATCH 1/2] ACPI/IORT: Handle potential overflow in iort_dma_setup [not found] ` <20181219131849.hziujd5zgclangce-gVz1Vyx/EOXkZJWtSm8s3NvLeJWuRmrY@public.gmane.org> @ 2019-01-10 10:44 ` Auger Eric [not found] ` <fbf8dc04-6f80-b30b-c9ef-87fa3a33d0ec-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Auger Eric @ 2019-01-10 10:44 UTC (permalink / raw) To: Andrew Jones, Robin Murphy Cc: linux-acpi-u79uwXL29TY76Z2rM5mHXA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, sudeep.holla-5wv7dgnIgG8 Hi Robin, Drew, On 12/19/18 2:18 PM, Andrew Jones wrote: > On Wed, Dec 19, 2018 at 12:21:35PM +0000, Robin Murphy wrote: >> On 18/12/2018 18:48, Andrew Jones wrote: >>> The sum of dmaaddr and size may overflow, particularly considering >>> there are cases where size will be U64_MAX. >> >> Only if the firmware is broken in the first place, though. It would be weird >> to describe an explicit _DMA range of base=0 and size=U64_MAX, because it's >> effectively the same as just not having one at all, but it's not strictly >> illegal. However, since the ACPI System Memory address space is at most >> 64-bit, anything that would actually overflow here is already describing an >> impossibility - really, we should probably scream even louder about a >> firmware bug and reject it entirely, rather than quietly hiding it. > > Ah, looking again I see the paths. Either acpi_dma_get_range() returns > success, in which case base and size are fine, or it returns an EINVAL, > in which case base=size=0, or it returns ENODEV in which case base is > zero, so size may be set to U64_MAX by rc_dma_get_range() with no problem. > The !dev_is_pci(dev) path is also fine since base=0. So practically putting an explicit memory_address_limit=64 is harmless as dmaaddr always is 0, right? In QEMU I intended to update the ACPI code to comply with the rev D spec. in that case the RC table revision is 1 (rev D) and the memory_address_limit needs to be filled. If we don't want to restrict the limit, isn't it the right choice to set 64 here? Thanks Eric > > While I agree that we should complain when firmware provides bad ACPI > tables, my understanding of setting iort.memory_address_limit=64 was > that it simply meant "no limit", regardless of the base. However I now > see that it won't be used unless base=0. So I don't think we have a > problem, and we don't even seem to be missing firmware sanity checks. > > It might be nice to have a comment explaining this stuff somewhere, but > otherwise sorry for the noise. > > Thanks, > drew > >> >> Robin. >> >>> Signed-off-by: Andrew Jones <drjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> >>> --- >>> drivers/acpi/arm64/iort.c | 7 ++++++- >>> 1 file changed, 6 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c >>> index 70f4e80b9246..a0f4c157ba5e 100644 >>> --- a/drivers/acpi/arm64/iort.c >>> +++ b/drivers/acpi/arm64/iort.c >>> @@ -1002,7 +1002,12 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size) >>> } >>> if (!ret) { >>> - msb = fls64(dmaaddr + size - 1); >>> + u64 dmaaddr_max = dmaaddr + size - 1; >>> + if (dmaaddr_max >= dmaaddr) >>> + msb = fls64(dmaaddr_max); >>> + else >>> + msb = 64; >>> + >>> /* >>> * Round-up to the power-of-two mask or set >>> * the mask to the whole 64-bit address space >>> ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <fbf8dc04-6f80-b30b-c9ef-87fa3a33d0ec-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH 1/2] ACPI/IORT: Handle potential overflow in iort_dma_setup [not found] ` <fbf8dc04-6f80-b30b-c9ef-87fa3a33d0ec-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2019-01-14 11:10 ` Robin Murphy [not found] ` <7acdfce6-0a0b-bf68-c5ff-8979721f4e83-5wv7dgnIgG8@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Robin Murphy @ 2019-01-14 11:10 UTC (permalink / raw) To: Auger Eric, Andrew Jones Cc: linux-acpi-u79uwXL29TY76Z2rM5mHXA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, sudeep.holla-5wv7dgnIgG8 On 10/01/2019 10:44, Auger Eric wrote: > Hi Robin, Drew, > > On 12/19/18 2:18 PM, Andrew Jones wrote: >> On Wed, Dec 19, 2018 at 12:21:35PM +0000, Robin Murphy wrote: >>> On 18/12/2018 18:48, Andrew Jones wrote: >>>> The sum of dmaaddr and size may overflow, particularly considering >>>> there are cases where size will be U64_MAX. >>> >>> Only if the firmware is broken in the first place, though. It would be weird >>> to describe an explicit _DMA range of base=0 and size=U64_MAX, because it's >>> effectively the same as just not having one at all, but it's not strictly >>> illegal. However, since the ACPI System Memory address space is at most >>> 64-bit, anything that would actually overflow here is already describing an >>> impossibility - really, we should probably scream even louder about a >>> firmware bug and reject it entirely, rather than quietly hiding it. >> >> Ah, looking again I see the paths. Either acpi_dma_get_range() returns >> success, in which case base and size are fine, or it returns an EINVAL, >> in which case base=size=0, or it returns ENODEV in which case base is >> zero, so size may be set to U64_MAX by rc_dma_get_range() with no problem. >> The !dev_is_pci(dev) path is also fine since base=0. > > So practically putting an explicit memory_address_limit=64 is harmless > as dmaaddr always is 0, right? > > In QEMU I intended to update the ACPI code to comply with the rev D > spec. in that case the RC table revision is 1 (rev D) and the > memory_address_limit needs to be filled. If we don't want to restrict > the limit, isn't it the right choice to set 64 here? Indeed, the Memory Address Size Limit doesn't cater for offsets so can't run into this kind of overflow in the first place. For a fully-emulated PCI hierarchy I'd say 64 is not just harmless but in fact entirely correct - you're going to have more fun with VFIO passthrough if the host tables have more restrictive limits, but I guess that's a problem for the future ;) Robin. ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <7acdfce6-0a0b-bf68-c5ff-8979721f4e83-5wv7dgnIgG8@public.gmane.org>]
* Re: [PATCH 1/2] ACPI/IORT: Handle potential overflow in iort_dma_setup [not found] ` <7acdfce6-0a0b-bf68-c5ff-8979721f4e83-5wv7dgnIgG8@public.gmane.org> @ 2019-01-14 15:29 ` Auger Eric 0 siblings, 0 replies; 9+ messages in thread From: Auger Eric @ 2019-01-14 15:29 UTC (permalink / raw) To: Robin Murphy, Andrew Jones Cc: linux-acpi-u79uwXL29TY76Z2rM5mHXA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, sudeep.holla-5wv7dgnIgG8 Hi Robin, On 1/14/19 12:10 PM, Robin Murphy wrote: > On 10/01/2019 10:44, Auger Eric wrote: >> Hi Robin, Drew, >> >> On 12/19/18 2:18 PM, Andrew Jones wrote: >>> On Wed, Dec 19, 2018 at 12:21:35PM +0000, Robin Murphy wrote: >>>> On 18/12/2018 18:48, Andrew Jones wrote: >>>>> The sum of dmaaddr and size may overflow, particularly considering >>>>> there are cases where size will be U64_MAX. >>>> >>>> Only if the firmware is broken in the first place, though. It would >>>> be weird >>>> to describe an explicit _DMA range of base=0 and size=U64_MAX, >>>> because it's >>>> effectively the same as just not having one at all, but it's not >>>> strictly >>>> illegal. However, since the ACPI System Memory address space is at most >>>> 64-bit, anything that would actually overflow here is already >>>> describing an >>>> impossibility - really, we should probably scream even louder about a >>>> firmware bug and reject it entirely, rather than quietly hiding it. >>> >>> Ah, looking again I see the paths. Either acpi_dma_get_range() returns >>> success, in which case base and size are fine, or it returns an EINVAL, >>> in which case base=size=0, or it returns ENODEV in which case base is >>> zero, so size may be set to U64_MAX by rc_dma_get_range() with no >>> problem. >>> The !dev_is_pci(dev) path is also fine since base=0. >> >> So practically putting an explicit memory_address_limit=64 is harmless >> as dmaaddr always is 0, right? >> >> In QEMU I intended to update the ACPI code to comply with the rev D >> spec. in that case the RC table revision is 1 (rev D) and the >> memory_address_limit needs to be filled. If we don't want to restrict >> the limit, isn't it the right choice to set 64 here? > > Indeed, the Memory Address Size Limit doesn't cater for offsets so can't > run into this kind of overflow in the first place. For a fully-emulated > PCI hierarchy I'd say 64 is not just harmless but in fact entirely > correct - you're going to have more fun with VFIO passthrough if the > host tables have more restrictive limits, but I guess that's a problem > for the future ;) Yes but that's a good point to notice! Thanks Eric > > Robin. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] iommu/dma: Handle potential overflow in iommu_dma_init_domain [not found] ` <20181218184841.20034-1-drjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2018-12-18 18:48 ` [PATCH 1/2] ACPI/IORT: Handle potential overflow in iort_dma_setup Andrew Jones @ 2018-12-18 18:48 ` Andrew Jones [not found] ` <20181218184841.20034-3-drjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 1 sibling, 1 reply; 9+ messages in thread From: Andrew Jones @ 2018-12-18 18:48 UTC (permalink / raw) To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-acpi-u79uwXL29TY76Z2rM5mHXA Cc: sudeep.holla-5wv7dgnIgG8, robin.murphy-5wv7dgnIgG8 The sum of base and size may overflow, particularly considering there are cases where size will be U64_MAX. Also, end_pfn is unused, so we remove it. Finally, as size doesn't actually need to be IOMMU page aligned we remove it from the comment stating both it and base should be. I wonder if we shouldn't at least warn when base is not aligned? Signed-off-by: Andrew Jones <drjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> --- drivers/iommu/dma-iommu.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index d1b04753b204..a0b01398b15c 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -281,9 +281,9 @@ static void iommu_dma_flush_iotlb_all(struct iova_domain *iovad) * @size: Size of IOVA space * @dev: Device the domain is being initialised for * - * @base and @size should be exact multiples of IOMMU page granularity to - * avoid rounding surprises. If necessary, we reserve the page at address 0 - * to ensure it is an invalid IOVA. It is safe to reinitialise a domain, but + * @base should be an exact multiple of IOMMU page granularity to avoid + * rounding surprises. If necessary, we reserve the page at address 0 to + * ensure it is an invalid IOVA. It is safe to reinitialise a domain, but * any change which could make prior IOVAs invalid will fail. */ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, @@ -291,21 +291,24 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, { struct iommu_dma_cookie *cookie = domain->iova_cookie; struct iova_domain *iovad = &cookie->iovad; - unsigned long order, base_pfn, end_pfn; + dma_addr_t max_addr = base + size - 1; + unsigned long order, base_pfn; int attr; if (!cookie || cookie->type != IOMMU_DMA_IOVA_COOKIE) return -EINVAL; + if (max_addr < base) + max_addr = U64_MAX; + /* Use the smallest supported page size for IOVA granularity */ order = __ffs(domain->pgsize_bitmap); base_pfn = max_t(unsigned long, 1, base >> order); - end_pfn = (base + size - 1) >> order; /* Check the domain allows at least some access to the device... */ if (domain->geometry.force_aperture) { if (base > domain->geometry.aperture_end || - base + size <= domain->geometry.aperture_start) { + max_addr < domain->geometry.aperture_start) { pr_warn("specified DMA range outside IOMMU capability\n"); return -EFAULT; } -- 2.17.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
[parent not found: <20181218184841.20034-3-drjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH 2/2] iommu/dma: Handle potential overflow in iommu_dma_init_domain [not found] ` <20181218184841.20034-3-drjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2018-12-19 13:02 ` Robin Murphy 0 siblings, 0 replies; 9+ messages in thread From: Robin Murphy @ 2018-12-19 13:02 UTC (permalink / raw) To: Andrew Jones, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-acpi-u79uwXL29TY76Z2rM5mHXA Cc: sudeep.holla-5wv7dgnIgG8 On 18/12/2018 18:48, Andrew Jones wrote: > The sum of base and size may overflow, particularly considering there > are cases where size will be U64_MAX. Also, end_pfn is unused, so we > remove it. Finally, as size doesn't actually need to be IOMMU page > aligned we remove it from the comment stating both it and base should > be. I wonder if we shouldn't at least warn when base is not aligned? TBH if we're going to do anything here we may as well just get rid of size altogether. It's pretty unrealistic that the check it's used in would ever actually fail, and even if a sufficiently weird system did exist for that to happen, I don't think it would make much practical difference to just carry on at this point and let DMA mapping calls fail later. Robin. > > Signed-off-by: Andrew Jones <drjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > --- > drivers/iommu/dma-iommu.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > index d1b04753b204..a0b01398b15c 100644 > --- a/drivers/iommu/dma-iommu.c > +++ b/drivers/iommu/dma-iommu.c > @@ -281,9 +281,9 @@ static void iommu_dma_flush_iotlb_all(struct iova_domain *iovad) > * @size: Size of IOVA space > * @dev: Device the domain is being initialised for > * > - * @base and @size should be exact multiples of IOMMU page granularity to > - * avoid rounding surprises. If necessary, we reserve the page at address 0 > - * to ensure it is an invalid IOVA. It is safe to reinitialise a domain, but > + * @base should be an exact multiple of IOMMU page granularity to avoid > + * rounding surprises. If necessary, we reserve the page at address 0 to > + * ensure it is an invalid IOVA. It is safe to reinitialise a domain, but > * any change which could make prior IOVAs invalid will fail. > */ > int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, > @@ -291,21 +291,24 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, > { > struct iommu_dma_cookie *cookie = domain->iova_cookie; > struct iova_domain *iovad = &cookie->iovad; > - unsigned long order, base_pfn, end_pfn; > + dma_addr_t max_addr = base + size - 1; > + unsigned long order, base_pfn; > int attr; > > if (!cookie || cookie->type != IOMMU_DMA_IOVA_COOKIE) > return -EINVAL; > > + if (max_addr < base) > + max_addr = U64_MAX; > + > /* Use the smallest supported page size for IOVA granularity */ > order = __ffs(domain->pgsize_bitmap); > base_pfn = max_t(unsigned long, 1, base >> order); > - end_pfn = (base + size - 1) >> order; > > /* Check the domain allows at least some access to the device... */ > if (domain->geometry.force_aperture) { > if (base > domain->geometry.aperture_end || > - base + size <= domain->geometry.aperture_start) { > + max_addr < domain->geometry.aperture_start) { > pr_warn("specified DMA range outside IOMMU capability\n"); > return -EFAULT; > } > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-01-14 15:29 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-18 18:48 [PATCH 0/2] ACPI/IORT: handle potential overflows Andrew Jones [not found] ` <20181218184841.20034-1-drjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2018-12-18 18:48 ` [PATCH 1/2] ACPI/IORT: Handle potential overflow in iort_dma_setup Andrew Jones [not found] ` <20181218184841.20034-2-drjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2018-12-19 12:21 ` Robin Murphy [not found] ` <1503e3b8-1a6c-3b66-fa1e-d13f4e19f31f-5wv7dgnIgG8@public.gmane.org> 2018-12-19 13:18 ` Andrew Jones [not found] ` <20181219131849.hziujd5zgclangce-gVz1Vyx/EOXkZJWtSm8s3NvLeJWuRmrY@public.gmane.org> 2019-01-10 10:44 ` Auger Eric [not found] ` <fbf8dc04-6f80-b30b-c9ef-87fa3a33d0ec-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2019-01-14 11:10 ` Robin Murphy [not found] ` <7acdfce6-0a0b-bf68-c5ff-8979721f4e83-5wv7dgnIgG8@public.gmane.org> 2019-01-14 15:29 ` Auger Eric 2018-12-18 18:48 ` [PATCH 2/2] iommu/dma: Handle potential overflow in iommu_dma_init_domain Andrew Jones [not found] ` <20181218184841.20034-3-drjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2018-12-19 13:02 ` Robin Murphy
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.