Re: [Xen-devel] [PATCH FOR-4.10] xen/arm: guest_walk: Fix check again the IPS

[Stefano Stabellini]

On Tue, 10 Oct 2017, Julien Grall wrote:
> The function get_ipa_output_size is check whether the input size
> configured by the guest is valid and will return it.
> 
> The check is done with the IPS already shifted against
> TCR_EL1_IPS_48_BIT. However the constant has been defined with the
> shift included, resulting the check always been false.
> 
> Fix it by doing the check on the non-shifted value.
> 
> This was introduced by commit 7d623b358a "arm/mem_access: Add long-descriptor
> based gpt" introduced software page-table walk for stage-1.
> 
> Coverity-ID: 1457707
> Signed-off-by: Julien Grall 
> 
> ---
> 
> Cc: Sergej Proskurin 
> ---
>  xen/arch/arm/guest_walk.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c
> index c38bedcf65..a6de325572 100644
> --- a/xen/arch/arm/guest_walk.c
> +++ b/xen/arch/arm/guest_walk.c
> @@ -185,7 +185,7 @@ static int guest_walk_sd(const struct vcpu *v,
>  static int get_ipa_output_size(struct domain *d, register_t tcr,
> unsigned int *output_size)
>  {
> -unsigned int ips;
> +register_t ips;
>  
>  static const unsigned int ipa_sizes[7] = {
>  TCR_EL1_IPS_32_BIT_VAL,
> @@ -200,7 +200,7 @@ static int get_ipa_output_size(struct domain *d, 
> register_t tcr,
>  if ( is_64bit_domain(d) )
>  {
>  /* Get the intermediate physical address size. */
> -ips = (tcr & TCR_EL1_IPS_MASK) >> TCR_EL1_IPS_SHIFT;
> +ips = tcr & TCR_EL1_IPS_MASK;
>  
>  /*
>   * Return an error on reserved IPA output-sizes and if the IPA
> @@ -211,7 +211,7 @@ static int get_ipa_output_size(struct domain *d, 
> register_t tcr,
>  if ( ips > TCR_EL1_IPS_48_BIT )
>  return -EFAULT;
>  
> -*output_size = ipa_sizes[ips];
> +*output_size = ipa_sizes[ips >> TCR_EL1_IPS_SHIFT];
>  }
>  else
>  *output_size = TCR_EL1_IPS_40_BIT_VAL;

on arm32, I get:

guest_walk.c: In function 'get_ipa_output_size':
guest_walk.c:214:9: error: right shift count >= width of type [-Werror]
 *output_size = ipa_sizes[ips >> TCR_EL1_IPS_SHIFT];
 ^
cc1: all warnings being treated as errors

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH FOR-4.10] xen/arm: guest_walk: Fix check again the IPS

[Sergej Proskurin]

Hi Julien,


On 10/10/2017 05:20 PM, Julien Grall wrote:
> The function get_ipa_output_size is check whether the input size
> configured by the guest is valid and will return it.
>
> The check is done with the IPS already shifted against
> TCR_EL1_IPS_48_BIT. However the constant has been defined with the
> shift included, resulting the check always been false.

Good fix, thank you!

>
> Fix it by doing the check on the non-shifted value.
>
> This was introduced by commit 7d623b358a "arm/mem_access: Add long-descriptor
> based gpt" introduced software page-table walk for stage-1.
>
> Coverity-ID: 1457707
> Signed-off-by: Julien Grall 
>
> ---
>
> Cc: Sergej Proskurin 

Acked-by: Sergej Proskurin 

Thanks,
~Sergej

> ---
>  xen/arch/arm/guest_walk.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c
> index c38bedcf65..a6de325572 100644
> --- a/xen/arch/arm/guest_walk.c
> +++ b/xen/arch/arm/guest_walk.c
> @@ -185,7 +185,7 @@ static int guest_walk_sd(const struct vcpu *v,
>  static int get_ipa_output_size(struct domain *d, register_t tcr,
> unsigned int *output_size)
>  {
> -unsigned int ips;
> +register_t ips;
>  
>  static const unsigned int ipa_sizes[7] = {
>  TCR_EL1_IPS_32_BIT_VAL,
> @@ -200,7 +200,7 @@ static int get_ipa_output_size(struct domain *d, 
> register_t tcr,
>  if ( is_64bit_domain(d) )
>  {
>  /* Get the intermediate physical address size. */
> -ips = (tcr & TCR_EL1_IPS_MASK) >> TCR_EL1_IPS_SHIFT;
> +ips = tcr & TCR_EL1_IPS_MASK;
>  
>  /*
>   * Return an error on reserved IPA output-sizes and if the IPA
> @@ -211,7 +211,7 @@ static int get_ipa_output_size(struct domain *d, 
> register_t tcr,
>  if ( ips > TCR_EL1_IPS_48_BIT )
>  return -EFAULT;
>  
> -*output_size = ipa_sizes[ips];
> +*output_size = ipa_sizes[ips >> TCR_EL1_IPS_SHIFT];
>  }
>  else
>  *output_size = TCR_EL1_IPS_40_BIT_VAL;

-- 
Sergej Proskurin, M.Sc.
Wissenschaftlicher Mitarbeiter

Technische Universität München
Fakultät für Informatik
Lehrstuhl für Sicherheit in der Informatik

Boltzmannstraße 3
85748 Garching (bei München)

Tel. +49 (0)89 289-18592
Fax +49 (0)89 289-18579


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH FOR-4.10] xen/arm: guest_walk: Fix check again the IPS

[Julien Grall]

The function get_ipa_output_size is check whether the input size
configured by the guest is valid and will return it.

The check is done with the IPS already shifted against
TCR_EL1_IPS_48_BIT. However the constant has been defined with the
shift included, resulting the check always been false.

Fix it by doing the check on the non-shifted value.

This was introduced by commit 7d623b358a "arm/mem_access: Add long-descriptor
based gpt" introduced software page-table walk for stage-1.

Coverity-ID: 1457707
Signed-off-by: Julien Grall 

---

Cc: Sergej Proskurin 
---
 xen/arch/arm/guest_walk.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c
index c38bedcf65..a6de325572 100644
--- a/xen/arch/arm/guest_walk.c
+++ b/xen/arch/arm/guest_walk.c
@@ -185,7 +185,7 @@ static int guest_walk_sd(const struct vcpu *v,
 static int get_ipa_output_size(struct domain *d, register_t tcr,
unsigned int *output_size)
 {
-unsigned int ips;
+register_t ips;
 
 static const unsigned int ipa_sizes[7] = {
 TCR_EL1_IPS_32_BIT_VAL,
@@ -200,7 +200,7 @@ static int get_ipa_output_size(struct domain *d, register_t 
tcr,
 if ( is_64bit_domain(d) )
 {
 /* Get the intermediate physical address size. */
-ips = (tcr & TCR_EL1_IPS_MASK) >> TCR_EL1_IPS_SHIFT;
+ips = tcr & TCR_EL1_IPS_MASK;
 
 /*
  * Return an error on reserved IPA output-sizes and if the IPA
@@ -211,7 +211,7 @@ static int get_ipa_output_size(struct domain *d, register_t 
tcr,
 if ( ips > TCR_EL1_IPS_48_BIT )
 return -EFAULT;
 
-*output_size = ipa_sizes[ips];
+*output_size = ipa_sizes[ips >> TCR_EL1_IPS_SHIFT];
 }
 else
 *output_size = TCR_EL1_IPS_40_BIT_VAL;
-- 
2.11.0


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel