I can take this from here. I see an opportunity here to implement a nice optimization for while loops. We currently allocate and free a control state for each iteration of the loop. This is overkill, we should just reuse the existing control state and only delete it when the loop terminates. Once this is done, we can add the loop counter to the control state to implement a loop counter on a per-while basis. Bob >-----Original Message----- >From: linux-acpi-owner@xxxxxxxxxxxxxxx [mailto:linux-acpi- >owner@xxxxxxxxxxxxxxx] On Behalf Of Moore, Robert >Sent: Thursday, October 09, 2008 7:56 AM >To: Alexey Starikovskiy; LenBrown >Cc: Linux-acpi@xxxxxxxxxxxxxxx >Subject: RE: [PATCH] ACPICA: Add hard limit to while loop > >The patch below will allow only 64K loop executions in a single control >method, period -- regardless of the number of different loops or if there >are nested loops. Do we really want this? Or do we want to implement it >such that any single loop cannot execute more than 64K times? > >To implement the limit on a strict per-while basis, the loop counter should >be put into the walk_state->control_state. A control_state is allocated for >each while. > >Bob > > > > >>-----Original Message----- >>From: linux-acpi-owner@xxxxxxxxxxxxxxx [mailto:linux-acpi- >>owner@xxxxxxxxxxxxxxx] On Behalf Of Alexey Starikovskiy >>Sent: Thursday, October 09, 2008 3:36 AM >>To: LenBrown >>Cc: Linux-acpi@xxxxxxxxxxxxxxx >>Subject: [PATCH] ACPICA: Add hard limit to while loop >> >>We need to prevent infinite loops and lockups >> >>Signed-off-by: Alexey Starikovskiy <astarikovskiy@xxxxxxx> >>--- >> >> drivers/acpi/dispatcher/dsopcode.c | 9 +++++++-- >> include/acpi/acstruct.h | 1 + >> 2 files changed, 8 insertions(+), 2 deletions(-) >> >> >>diff --git a/drivers/acpi/dispatcher/dsopcode.c >>b/drivers/acpi/dispatcher/dsopcode.c >>index 6a81c44..e2a3535 100644 >>--- a/drivers/acpi/dispatcher/dsopcode.c >>+++ b/drivers/acpi/dispatcher/dsopcode.c >>@@ -1246,8 +1246,13 @@ acpi_ds_exec_end_control_op(struct acpi_walk_state >* >>walk_state, >> if (walk_state->control_state->common.value) { >> >> /* Predicate was true, go back and evaluate it >>again! */ >>- >>- status = AE_CTRL_PENDING; >>+ /* Use hard limit to prevent infinite loops and >>lockups */ >>+ if (++walk_state->cycle_count > 0xFFFF) { >>+ status = AE_LIMIT; >>+ ACPI_ERROR ((AE_INFO, "Infinite loop >>detected")); >>+ } else { >>+ status = AE_CTRL_PENDING; >>+ } >> } >> >> ACPI_DEBUG_PRINT((ACPI_DB_DISPATCH, >>diff --git a/include/acpi/acstruct.h b/include/acpi/acstruct.h >>index 7980a26..fb4405e 100644 >>--- a/include/acpi/acstruct.h >>+++ b/include/acpi/acstruct.h >>@@ -94,6 +94,7 @@ struct acpi_walk_state { >> u32 method_breakpoint; /* For single stepping */ >> u32 user_breakpoint; /* User AML breakpoint */ >> u32 parse_flags; >>+ u32 cycle_count; /* While loop cycle count */ >> >> struct acpi_parse_state parser_state; /* Current state of parser >>*/ >> u32 prev_arg_types; >> >>-- >>To unsubscribe from this list: send the line "unsubscribe linux-acpi" in >>the body of a message to majordomo@xxxxxxxxxxxxxxx >>More majordomo info at http://vger.kernel.org/majordomo-info.html >-- >To unsubscribe from this list: send the line "unsubscribe linux-acpi" in >the body of a message to majordomo@xxxxxxxxxxxxxxx >More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
