Re: [Xen-devel] [PATCH v2] fix potential pa_range_info out of bound access

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 12 Dec 2016, Julien Grall wrote:
> On 12/12/16 19:17, Stefano Stabellini wrote:
> > On Mon, 12 Dec 2016, Julien Grall wrote:
> > > Hi Stefano,
> > > 
> > > On 09/12/16 19:46, 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 checking that pa_range is not greater
> > > > than the size of the array.
> > > > 
> > > > 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..eb791db 100644
> > > > --- a/xen/arch/arm/p2m.c
> > > > +++ b/xen/arch/arm/p2m.c
> > > > @@ -1639,7 +1639,8 @@ void __init setup_virt_paging(void)
> > > >      }
> > > > 
> > > >      /* pa_range is 4 bits, but the defined encodings are only 3 bits */
> > > > -    if ( pa_range&0x8 || !pa_range_info[pa_range].pabits )
> > > > +    if ( pa_range >= ARRAY_SIZE(pa_range_info) ||
> > > > +         pa_range&0x8 || !pa_range_info[pa_range].pabits )
> > > 
> > > I don't see any valid reason to check whether bit 3 is set (e.g the check
> > > "pa_range&0x8"). IHMO it was a bad way to check if pa_range was in the
> > > array
> > > index boundary.
> > 
> > Given the comment, I think the author probably meant pa_range&0x7, but
> > the new check is better. I'll remove it.
> 
> You can add my reviewed-by to avoid another round on the ML.

Thanks, I'll do

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