diff mbox series

[Xen-devel,v2] xen/arm: Fix the issue in cmp_mmio_handler used in find_mmio_handler

Message ID 1506645206-7184-1-git-send-email-bhupinder.thakur@linaro.org
State Superseded
Headers show
Series [Xen-devel,v2] xen/arm: Fix the issue in cmp_mmio_handler used in find_mmio_handler | expand

Commit Message

Bhupinder Thakur Sept. 29, 2017, 12:33 a.m. UTC
This function returns true/false based on whether the key value
is in the range (start, start+size). However, it should check against
(start, start+size-1) because start+size falls outside the range.

This resulted in returning a wrong mmio_handler for a given mmio address which
happened to be start+size.

This bug was introduced when the mmio region search was switched from
linear search to binary search in the following commit:

8047e09 "xen/arm: io: Use binary search for mmio handler lookup".

This change may have to be back-ported to 4.8 also.

Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
---
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>

 xen/arch/arm/io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Julien Grall Sept. 29, 2017, 12:43 a.m. UTC | #1
Hi Bhupinder,

On 09/29/2017 01:33 AM, Bhupinder Thakur wrote:
> This function returns true/false based on whether the key value

Technically, this function returns 3 values: -1/0/1. The commit message 
should be updated to reflect that.

> is in the range (start, start+size). However, it should check against
> (start, start+size-1) because start+size falls outside the range.
> 
> This resulted in returning a wrong mmio_handler for a given mmio address which
> happened to be start+size.
> 
> This bug was introduced when the mmio region search was switched from
> linear search to binary search in the following commit:
> 
> 8047e09 "xen/arm: io: Use binary search for mmio handler lookup".
> 
> This change may have to be back-ported to 4.8 also.

FIY, this does not need to be in the commit message. You could add after 
"---". When committed, it will get removed.

> 
> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
> ---
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien.grall@arm.com>
> 
>   xen/arch/arm/io.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
> index e216128..c748d8f 100644
> --- a/xen/arch/arm/io.c
> +++ b/xen/arch/arm/io.c
> @@ -79,7 +79,7 @@ static int cmp_mmio_handler(const void *key, const void *elem)
>       if ( handler0->addr < handler1->addr )
>           return -1;
>   
> -    if ( handler0->addr > (handler1->addr + handler1->size) )
> +    if ( handler0->addr >= (handler1->addr + handler1->size) )
>           return 1;
>   
>       return 0;
> 

Cheers,
diff mbox series

Patch

diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
index e216128..c748d8f 100644
--- a/xen/arch/arm/io.c
+++ b/xen/arch/arm/io.c
@@ -79,7 +79,7 @@  static int cmp_mmio_handler(const void *key, const void *elem)
     if ( handler0->addr < handler1->addr )
         return -1;
 
-    if ( handler0->addr > (handler1->addr + handler1->size) )
+    if ( handler0->addr >= (handler1->addr + handler1->size) )
         return 1;
 
     return 0;