On Thu, 28 Apr 2016 16:55:13 +1000
Balbir Singh <[email protected]> wrote:

> On 28/04/16 16:17, Suraj Jitindar Singh wrote:
> > When unregistering a crash_shutdown_handle in the function
> > crash_shutdown_unregister() the other handles are shifted down in
> > the array to replace the unregistered handle. The for loop assumes
> > that the last element in the array is null and uses this as the
> > stop condition, however in the case that the last element is not
> > null there is no check to ensure that an out of bounds access is
> > not performed.
> > 
> > Add a check to terminate the shift operation when CRASH_HANDLER_MAX
> > is reached in order to protect against out of bounds accesses.
> > 
> > Signed-off-by: Suraj Jitindar Singh <[email protected]>
> > ---
> >  arch/powerpc/kernel/crash.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/powerpc/kernel/crash.c
> > b/arch/powerpc/kernel/crash.c index 2bb252c..6b267af 100644
> > --- a/arch/powerpc/kernel/crash.c
> > +++ b/arch/powerpc/kernel/crash.c
> > @@ -288,7 +288,7 @@ int crash_shutdown_unregister(crash_shutdown_t
> > handler) rc = 1;
> >     } else {
> >             /* Shift handles down */
> > -           for (; crash_shutdown_handles[i]; i++)
> > +           for (; crash_shutdown_handles[i] && i <
> > CRASH_HANDLER_MAX; i++) crash_shutdown_handles[i] =
> >                             crash_shutdown_handles[i+1];
> >             rc = 0;
> >   
> 
> with i = CRASH_HANDLER_MAX-1 we could end up with
> crash_shutdown_handles[i+1] already out of bounds I think you need to
> check that i+1 does not overflow
> 
> Balbir

Thanks for taking a look Balbir, the size of crash_shutdown_handles is
actually CRASH_HANDLER_MAX+1.
_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to