| Message ID | 20171010152034.6388-1-julien.grall@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <julien.grall@linaro.org> > > --- > > Cc: Sergej Proskurin <proskurin@sec.in.tum.de> Acked-by: Sergej Proskurin <proskurin@sec.in.tum.de> 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;
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 <julien.grall@linaro.org> > > --- > > Cc: Sergej Proskurin <proskurin@sec.in.tum.de> > --- > 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
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;
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 <julien.grall@linaro.org> --- Cc: Sergej Proskurin <proskurin@sec.in.tum.de> --- xen/arch/arm/guest_walk.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)