Commit 1d145165 authored by Suraj Jitindar Singh's avatar Suraj Jitindar Singh Committed by Michael Ellerman

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: default avatarSuraj Jitindar Singh <sjitindarsingh@gmail.com>
Signed-off-by: default avatarMichael Ellerman <mpe@ellerman.id.au>
parent 3079abe5
...@@ -48,8 +48,8 @@ int crashing_cpu = -1; ...@@ -48,8 +48,8 @@ int crashing_cpu = -1;
static int time_to_dump; static int time_to_dump;
#define CRASH_HANDLER_MAX 3 #define CRASH_HANDLER_MAX 3
/* NULL terminated list of shutdown handles */ /* List of shutdown handles */
static crash_shutdown_t crash_shutdown_handles[CRASH_HANDLER_MAX+1]; static crash_shutdown_t crash_shutdown_handles[CRASH_HANDLER_MAX];
static DEFINE_SPINLOCK(crash_handlers_lock); static DEFINE_SPINLOCK(crash_handlers_lock);
static unsigned long crash_shutdown_buf[JMP_BUF_LEN]; static unsigned long crash_shutdown_buf[JMP_BUF_LEN];
...@@ -288,9 +288,14 @@ int crash_shutdown_unregister(crash_shutdown_t handler) ...@@ -288,9 +288,14 @@ int crash_shutdown_unregister(crash_shutdown_t handler)
rc = 1; rc = 1;
} else { } else {
/* Shift handles down */ /* Shift handles down */
for (; crash_shutdown_handles[i]; i++) for (; i < (CRASH_HANDLER_MAX - 1); i++)
crash_shutdown_handles[i] = crash_shutdown_handles[i] =
crash_shutdown_handles[i+1]; 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; rc = 0;
} }
...@@ -346,7 +351,7 @@ void default_machine_crash_shutdown(struct pt_regs *regs) ...@@ -346,7 +351,7 @@ void default_machine_crash_shutdown(struct pt_regs *regs)
old_handler = __debugger_fault_handler; old_handler = __debugger_fault_handler;
__debugger_fault_handler = handle_fault; __debugger_fault_handler = handle_fault;
crash_shutdown_cpu = smp_processor_id(); 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) { if (setjmp(crash_shutdown_buf) == 0) {
/* /*
* Insert syncs and delay to ensure * Insert syncs and delay to ensure
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment