[PATCH] x86/sysctl: Don't clobber memory if NCAPINTS > ARRAY_SIZE(pi->hw_cap)

[PATCH] x86/sysctl: Don't clobber memory if NCAPINTS > ARRAY_SIZE(pi->hw_cap)

[Andrew Cooper]

There is no current problem, as both NCAPINTS and pi->hw_cap are 8 entries,
but the limit should be calculated appropriately so as to avoid hypervisor
stack corruption if the two do get out of sync.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>

I came across this during my cpuid levelling work.  As I know I am not the
only person playing with NCAPINTS at the moment, I am posting this ahead of
the rest of the work.

Wei: Concerning 4.6, it might we worth taking this, as it will likely bite
downstream distributers who backport a 4.7 feature.

Also not fixed here is the fact that the libxl ABI hardcodes an 8 as the
length of this array, which is wrong.  I have insufficient tuits to come up
with a backwards compatible fix at this time.
---
 xen/arch/x86/sysctl.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index f36b52f..38b5dcb 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -75,7 +75,8 @@ long cpu_down_helper(void *data)
 
 void arch_do_physinfo(xen_sysctl_physinfo_t *pi)
 {
-    memcpy(pi->hw_cap, boot_cpu_data.x86_capability, NCAPINTS*4);
+    memcpy(pi->hw_cap, boot_cpu_data.x86_capability,
+           min(sizeof(pi->hw_cap), sizeof(boot_cpu_data.x86_capability)));
     if ( hvm_enabled )
         pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm;
     if ( iommu_enabled )
-- 
1.7.10.4

Re: [PATCH] x86/sysctl: Don't clobber memory if NCAPINTS > ARRAY_SIZE(pi->hw_cap)

[Wei Liu]

On Wed, Sep 16, 2015 at 10:01:45AM +0100, Andrew Cooper wrote:
> There is no current problem, as both NCAPINTS and pi->hw_cap are 8 entries,
> but the limit should be calculated appropriately so as to avoid hypervisor
> stack corruption if the two do get out of sync.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> 
> I came across this during my cpuid levelling work.  As I know I am not the
> only person playing with NCAPINTS at the moment, I am posting this ahead of
> the rest of the work.
> 
> Wei: Concerning 4.6, it might we worth taking this, as it will likely bite
> downstream distributers who backport a 4.7 feature.
> 

Release-acked-by: Wei Liu <wei.liu2@citrix.com>

> Also not fixed here is the fact that the libxl ABI hardcodes an 8 as the
> length of this array, which is wrong.  I have insufficient tuits to come up
> with a backwards compatible fix at this time.

Libxl only provides stable APIs not stable ABIs so maybe we can get away
with this? Anyway this is another topic and should be discussed
separately.

Wei.

> ---
>  xen/arch/x86/sysctl.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
> index f36b52f..38b5dcb 100644
> --- a/xen/arch/x86/sysctl.c
> +++ b/xen/arch/x86/sysctl.c
> @@ -75,7 +75,8 @@ long cpu_down_helper(void *data)
>  
>  void arch_do_physinfo(xen_sysctl_physinfo_t *pi)
>  {
> -    memcpy(pi->hw_cap, boot_cpu_data.x86_capability, NCAPINTS*4);
> +    memcpy(pi->hw_cap, boot_cpu_data.x86_capability,
> +           min(sizeof(pi->hw_cap), sizeof(boot_cpu_data.x86_capability)));
>      if ( hvm_enabled )
>          pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm;
>      if ( iommu_enabled )
> -- 
> 1.7.10.4

Re: [PATCH] x86/sysctl: Don't clobber memory if NCAPINTS > ARRAY_SIZE(pi->hw_cap)

[Ian Campbell]

On Wed, 2015-09-16 at 16:01 +0100, Wei Liu wrote:
> On Wed, Sep 16, 2015 at 10:01:45AM +0100, Andrew Cooper wrote:
> > There is no current problem, as both NCAPINTS and pi->hw_cap are 8
> > entries,
> > but the limit should be calculated appropriately so as to avoid
> > hypervisor
> > stack corruption if the two do get out of sync.
> > 
> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > ---
> > CC: Jan Beulich <JBeulich@suse.com>
> > CC: Wei Liu <wei.liu2@citrix.com>
> > 
> > I came across this during my cpuid levelling work.  As I know I am not
> > the
> > only person playing with NCAPINTS at the moment, I am posting this
> > ahead of
> > the rest of the work.
> > 
> > Wei: Concerning 4.6, it might we worth taking this, as it will likely
> > bite
> > downstream distributers who backport a 4.7 feature.
> > 
> 
> Release-acked-by: Wei Liu <wei.liu2@citrix.com>

Andy tells me that Jan is away so I have cherry-picked this
(c373b912e74659f0e0898ae93e89513694cfd94e) to staging-4.6 at his request.

Ian.