powerpc: Add array bounds checking to crash_shutdown_handlers

The array crash_shutdown_handles is an array of size CRASH_HANDLER_MAX+1
containing up to CRASH_HANDLER_MAX shutdown_handlers. It is assumed to
be NULL terminated, which it is under normal circumstances. Array
accesses in the functions crash_shutdown_unregister() and
default_machine_crash_shutdown() rely on this NULL termination property
when traversing this list and don't protect again out of bounds accesses.
If the NULL terminator were somehow overwritten these functions could
potentially access out of the bounds of the array.

Shrink the array to size CRASH_HANDLER_MAX and implement explicit array
bounds checking when accessing the elements of the
crash_shutdown_handles[] array in crash_shutdown_unregister() and
default_machine_crash_shutdown().

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

diff --git a/arch/powerpc/kernel/crash.c b/arch/powerpc/kernel/crash.c
index 2bb252c01f07c583a8bf9cb863853593b5224759..3dc1fade0b281c0c7c8f9bdfc5bc91c049638f4f 100644 (file)
--- a/arch/powerpc/kernel/crash.c
+++ b/arch/powerpc/kernel/crash.c
@@ -48,8 +48,8 @@ int crashing_cpu = -1;
 static int time_to_dump;
 
 #define CRASH_HANDLER_MAX 3
-/* NULL terminated list of shutdown handles */
-static crash_shutdown_t crash_shutdown_handles[CRASH_HANDLER_MAX+1];
+/* List of shutdown handles */
+static crash_shutdown_t crash_shutdown_handles[CRASH_HANDLER_MAX];
 static DEFINE_SPINLOCK(crash_handlers_lock);
 
 static unsigned long crash_shutdown_buf[JMP_BUF_LEN];
@@ -288,9 +288,14 @@ int crash_shutdown_unregister(crash_shutdown_t handler)
                rc = 1;
        } else {
                /* Shift handles down */
-               for (; crash_shutdown_handles[i]; i++)
+               for (; i < (CRASH_HANDLER_MAX - 1); i++)
                        crash_shutdown_handles[i] =
                                crash_shutdown_handles[i+1];
+               /*
+                * Reset last entry to NULL now that it has been shifted down,
+                * this will allow new handles to be added here.
+                */
+               crash_shutdown_handles[i] = NULL;
                rc = 0;
        }
 
@@ -346,7 +351,7 @@ void default_machine_crash_shutdown(struct pt_regs *regs)
        old_handler = __debugger_fault_handler;
        __debugger_fault_handler = handle_fault;
        crash_shutdown_cpu = smp_processor_id();
-       for (i = 0; crash_shutdown_handles[i]; i++) {
+       for (i = 0; crash_shutdown_handles[i] && i < CRASH_HANDLER_MAX; i++) {
                if (setjmp(crash_shutdown_buf) == 0) {
                        /*
                         * Insert syncs and delay to ensure