1. Patchew
  2. Xen
  3. [PATCH v2 0/2] xen/arm: fix iomem_ranges cfg in map_range_to_domain()
  4. View patch

[PATCH v2 1/2] xen/arm: fix iomem permissions cfg in map_range_to_domain()

Grygorii Strashko posted 2 patches 8 months, 2 weeks ago
[PATCH v2 1/2] xen/arm: fix iomem permissions cfg in map_range_to_domain()
Posted by Grygorii Strashko 8 months, 2 weeks ago 
Now the following code in map_range_to_domain()

    res = iomem_permit_access(d, paddr_to_pfn(addr),
                    paddr_to_pfn(PAGE_ALIGN(addr + len - 1)));

calculates the iomem range end address by rounding it up to the next Xen
page with incorrect assumption that iomem range end address passed to
iomem_permit_access() is exclusive, while it is expected to be inclusive.
It gives Control domain (Dom0) access to manage incorrect MMIO range with
one additional page.

For example, if requested range is [00e6140000:00e6141004] then it expected
to add [e6140:e6141] range (num_pages=2) to the domain iomem_caps rangeset,
but will add [e6140:e6142] (num_pages=3) instead.

To fix it, drop PAGE_ALIGN() from the iomem range end address calculation
formula.

Fixes: 33233c2758345 ("arch/arm: domain build: let dom0 access I/O memory
of mapped devices")
Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
---
 xen/arch/arm/device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/device.c b/xen/arch/arm/device.c
index 5610cddcba8e..97e613e06afa 100644
--- a/xen/arch/arm/device.c
+++ b/xen/arch/arm/device.c
@@ -71,7 +71,7 @@ int map_range_to_domain(const struct dt_device_node *dev,
                      strlen("/reserved-memory/")) != 0 )
     {
         res = iomem_permit_access(d, paddr_to_pfn(addr),
-                paddr_to_pfn(PAGE_ALIGN(addr + len - 1)));
+                                  paddr_to_pfn(addr + len - 1));
         if ( res )
         {
             printk(XENLOG_ERR "Unable to permit to dom%d access to"
-- 
2.34.1
Re: [PATCH v2 1/2] xen/arm: fix iomem permissions cfg in map_range_to_domain()
Posted by Julien Grall 8 months, 2 weeks ago 
Hi Grygorii,

On 18/02/2025 11:22, Grygorii Strashko wrote:
> Now the following code in map_range_to_domain()
> 
>      res = iomem_permit_access(d, paddr_to_pfn(addr),
>                      paddr_to_pfn(PAGE_ALIGN(addr + len - 1)));
> 
> calculates the iomem range end address by rounding it up to the next Xen
> page with incorrect assumption that iomem range end address passed to
> iomem_permit_access() is exclusive, while it is expected to be inclusive.
> It gives Control domain (Dom0) access to manage incorrect MMIO range with
> one additional page.
> 
> For example, if requested range is [00e6140000:00e6141004] then it expected
> to add [e6140:e6141] range (num_pages=2) to the domain iomem_caps rangeset,
> but will add [e6140:e6142] (num_pages=3) instead.
> 
> To fix it, drop PAGE_ALIGN() from the iomem range end address calculation
> formula.
> 
> Fixes: 33233c2758345 ("arch/arm: domain build: let dom0 access I/O memory
> of mapped devices")
> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>

Reviewed-by: Julien Grall <jgrall@amazon.com>

Cheers,

> ---
>   xen/arch/arm/device.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/device.c b/xen/arch/arm/device.c
> index 5610cddcba8e..97e613e06afa 100644
> --- a/xen/arch/arm/device.c
> +++ b/xen/arch/arm/device.c
> @@ -71,7 +71,7 @@ int map_range_to_domain(const struct dt_device_node *dev,
>                        strlen("/reserved-memory/")) != 0 )
>       {
>           res = iomem_permit_access(d, paddr_to_pfn(addr),
> -                paddr_to_pfn(PAGE_ALIGN(addr + len - 1)));
> +                                  paddr_to_pfn(addr + len - 1));
>           if ( res )
>           {
>               printk(XENLOG_ERR "Unable to permit to dom%d access to"

-- 
Julien Grall
Re: [PATCH v2 1/2] xen/arm: fix iomem permissions cfg in map_range_to_domain()
Posted by Grygorii Strashko 7 months, 3 weeks ago 
Hi

On 19.02.25 13:25, Julien Grall wrote:
> Hi Grygorii,
> 
> On 18/02/2025 11:22, Grygorii Strashko wrote:
>> Now the following code in map_range_to_domain()
>>
>>      res = iomem_permit_access(d, paddr_to_pfn(addr),
>>                      paddr_to_pfn(PAGE_ALIGN(addr + len - 1)));
>>
>> calculates the iomem range end address by rounding it up to the next Xen
>> page with incorrect assumption that iomem range end address passed to
>> iomem_permit_access() is exclusive, while it is expected to be inclusive.
>> It gives Control domain (Dom0) access to manage incorrect MMIO range with
>> one additional page.
>>
>> For example, if requested range is [00e6140000:00e6141004] then it expected
>> to add [e6140:e6141] range (num_pages=2) to the domain iomem_caps rangeset,
>> but will add [e6140:e6142] (num_pages=3) instead.
>>
>> To fix it, drop PAGE_ALIGN() from the iomem range end address calculation
>> formula.
>>
>> Fixes: 33233c2758345 ("arch/arm: domain build: let dom0 access I/O memory
>> of mapped devices")
>> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
> 
> Reviewed-by: Julien Grall <jgrall@amazon.com>

Sorry, that I'm disturbing you, but do i need to perform any additional actions here?

> 
> Cheers,
> 
>> ---
>>   xen/arch/arm/device.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/device.c b/xen/arch/arm/device.c
>> index 5610cddcba8e..97e613e06afa 100644
>> --- a/xen/arch/arm/device.c
>> +++ b/xen/arch/arm/device.c
>> @@ -71,7 +71,7 @@ int map_range_to_domain(const struct dt_device_node *dev,
>>                        strlen("/reserved-memory/")) != 0 )
>>       {
>>           res = iomem_permit_access(d, paddr_to_pfn(addr),
>> -                paddr_to_pfn(PAGE_ALIGN(addr + len - 1)));
>> +                                  paddr_to_pfn(addr + len - 1));
>>           if ( res )
>>           {
>>               printk(XENLOG_ERR "Unable to permit to dom%d access to"
> 

Best regards,
-grygorii
Re: [PATCH v2 1/2] xen/arm: fix iomem permissions cfg in map_range_to_domain()
Posted by Julien Grall 7 months, 3 weeks ago 

On 11/03/2025 11:31, Grygorii Strashko wrote:
> Hi
> 
> On 19.02.25 13:25, Julien Grall wrote:
>> Hi Grygorii,
>>
>> On 18/02/2025 11:22, Grygorii Strashko wrote:
>>> Now the following code in map_range_to_domain()
>>>
>>>      res = iomem_permit_access(d, paddr_to_pfn(addr),
>>>                      paddr_to_pfn(PAGE_ALIGN(addr + len - 1)));
>>>
>>> calculates the iomem range end address by rounding it up to the next Xen
>>> page with incorrect assumption that iomem range end address passed to
>>> iomem_permit_access() is exclusive, while it is expected to be 
>>> inclusive.
>>> It gives Control domain (Dom0) access to manage incorrect MMIO range 
>>> with
>>> one additional page.
>>>
>>> For example, if requested range is [00e6140000:00e6141004] then it 
>>> expected
>>> to add [e6140:e6141] range (num_pages=2) to the domain iomem_caps 
>>> rangeset,
>>> but will add [e6140:e6142] (num_pages=3) instead.
>>>
>>> To fix it, drop PAGE_ALIGN() from the iomem range end address 
>>> calculation
>>> formula.
>>>
>>> Fixes: 33233c2758345 ("arch/arm: domain build: let dom0 access I/O 
>>> memory
>>> of mapped devices")
>>> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
>>
>> Reviewed-by: Julien Grall <jgrall@amazon.com>
> 
> Sorry, that I'm disturbing you, but do i need to perform any additional 
> actions here?

Thanks for the reminder. The tree only re-opened a few days ago and I 
haven't had a chance to go through the list of pending patches. I have 
pushed them to a branch. If the CI pass, then I will push them to staging.

Cheers,

-- 
Julien Grall

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.