Re: [PATCH] powerpc: Add out of bounds check to crash_shutdown_unregister()

[Balbir Singh <bsingharora@gmail.com>]

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 <sjitindarsingh@gmail.com>
> ---
>  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