* [PATCH] fix potential pa_range_info out of bound access @ 2016-12-09 1:10 Stefano Stabellini 2016-12-09 1:40 ` Stefano Stabellini 0 siblings, 1 reply; 4+ messages in thread From: Stefano Stabellini @ 2016-12-09 1:10 UTC (permalink / raw) To: xen-devel; +Cc: julien.grall, sstabellini 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@kernel.org> 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 ) { _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] fix potential pa_range_info out of bound access 2016-12-09 1:10 [PATCH] fix potential pa_range_info out of bound access Stefano Stabellini @ 2016-12-09 1:40 ` Stefano Stabellini 2016-12-09 16:51 ` Julien Grall 0 siblings, 1 reply; 4+ messages in thread From: Stefano Stabellini @ 2016-12-09 1:40 UTC (permalink / raw) To: Stefano Stabellini; +Cc: xen-devel, julien.grall 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@kernel.org> > > 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@kernel.org> 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; for_each_online_cpu ( cpu ) { _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] fix potential pa_range_info out of bound access 2016-12-09 1:40 ` Stefano Stabellini @ 2016-12-09 16:51 ` Julien Grall 2016-12-09 19:44 ` Stefano Stabellini 0 siblings, 1 reply; 4+ messages in thread From: Julien Grall @ 2016-12-09 16:51 UTC (permalink / raw) To: Stefano Stabellini, Stefano Stabellini; +Cc: xen-devel 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@kernel.org> >> >> 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@kernel.org> > > 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) 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). Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fix potential pa_range_info out of bound access 2016-12-09 16:51 ` Julien Grall @ 2016-12-09 19:44 ` Stefano Stabellini 0 siblings, 0 replies; 4+ messages in thread From: Stefano Stabellini @ 2016-12-09 19:44 UTC (permalink / raw) To: Julien Grall; +Cc: Stefano Stabellini, xen-devel, Stefano Stabellini 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@kernel.org> > > > > > > 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@kernel.org> > > > > 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@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-12-09 19:44 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-12-09 1:10 [PATCH] fix potential pa_range_info out of bound access Stefano Stabellini 2016-12-09 1:40 ` Stefano Stabellini 2016-12-09 16:51 ` Julien Grall 2016-12-09 19:44 ` Stefano Stabellini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).