[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] fix potential pa_range_info out of bound access
On Fri, 9 Dec 2016, Julien Grall wrote: > Hi Stefano, > > On 09/12/16 01:40, Stefano Stabellini wrote: > > On Thu, 8 Dec 2016, Stefano Stabellini wrote: > > > pa_range_info has only 8 elements and is accessed using pa_range as > > > index. pa_range is initialized to 16, potentially causing out of bound > > > access errors. Fix the issue by initializing pa_range to the effective > > > number of pa_range_info elements. > > > > > > CID 1381865 > > > > > > Signed-off-by: Stefano Stabellini <sstabellini@xxxxxxxxxx> > > > > > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > > > index e4991df..245fcd1 100644 > > > --- a/xen/arch/arm/p2m.c > > > +++ b/xen/arch/arm/p2m.c > > > @@ -1629,7 +1629,7 @@ void __init setup_virt_paging(void) > > > }; > > > > > > unsigned int cpu; > > > - unsigned int pa_range = 0x10; /* Larger than any possible value */ > > > + unsigned int pa_range = sizeof(pa_range_info) / > > > sizeof(pa_range_info[0]); > > > > > > for_each_online_cpu ( cpu ) > > > { > > > > this is wrong, it should be sizeof(pa_range_info) / sizeof(pa_range_info[0]) > > - 1: > > > > --- > > pa_range_info has only 8 elements and is accessed using pa_range as > > index. pa_range is initialized to 16, potentially causing out of bound > > access errors. Fix the issue by initializing pa_range to the effective > > number of pa_range_info elements minus 1. > > > > Coverity-ID: 1381865 > > > > Signed-off-by: Stefano Stabellini <sstabellini@xxxxxxxxxx> > > > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > > index e4991df..14901b0 100644 > > --- a/xen/arch/arm/p2m.c > > +++ b/xen/arch/arm/p2m.c > > @@ -1629,7 +1629,7 @@ void __init setup_virt_paging(void) > > }; > > > > unsigned int cpu; > > - unsigned int pa_range = 0x10; /* Larger than any possible value */ > > + unsigned int pa_range = ARRAY_SIZE(pa_range_info) - 1; > > The previous value was confusing and I think this one is even more. > > But this is not really the problem, it is because the boundary check the later > on is wrong: > > if ( pa_range&0x8 || !pa_range_info[pa_range].pabits ) > > It will only check whether bit 3 is not set. But we want to check that > pa_range is the range of the array. I.e > > pa_range < ARRAY_SIZE(pa_range_info) You are right, that is better and I don't think it requires changing the initial value. Andrew suggested something similar on IRC too. > If you still want to change the pa_range initial value, then I would prefer to > see the boot CPU one (i.e boot_cpu_data.mm64.pa_range). _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |