From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758949AbcG0Fiz (ORCPT ); Wed, 27 Jul 2016 01:38:55 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:33998 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758807AbcG0Fin (ORCPT ); Wed, 27 Jul 2016 01:38:43 -0400 Subject: Re: [PATCH] ACPICA: cleanup method properly on error To: "Moore, Robert" , "Zheng, Lv" , "Wysocki, Rafael J" References: <1469201709-10518-1-git-send-email-vegard.nossum@oracle.com> <94F2FBAB4432B54E8AACC7DFDE6C92E37E4CD2A1@ORSMSX110.amr.corp.intel.com> <94F2FBAB4432B54E8AACC7DFDE6C92E37E4CDC04@ORSMSX110.amr.corp.intel.com> Cc: "linux-acpi@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "stable@vger.kernel.org" , "devel@acpica.org" From: Vegard Nossum Message-ID: <579848D9.9010900@oracle.com> Date: Wed, 27 Jul 2016 07:38:33 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: <94F2FBAB4432B54E8AACC7DFDE6C92E37E4CDC04@ORSMSX110.amr.corp.intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Source-IP: userv0021.oracle.com [156.151.31.71] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/26/2016 10:28 PM, Moore, Robert wrote: > /* Put the new state at the head of the walk list */ > > if (Thread) > { > AcpiDsPushWalkState (WalkState, Thread); > } > > Is there any chance that Thread could be zero? I'm not sure if this question was for me or not, but that code (modulo the capitalisation differences) is the reason why I also used 'if (thread)' in my patch. Vegard > >> -----Original Message----- >> From: Devel [mailto:devel-bounces@acpica.org] On Behalf Of Moore, Robert >> Sent: Tuesday, July 26, 2016 7:40 AM >> To: Vegard Nossum ; Zheng, Lv >> ; Wysocki, Rafael J >> Cc: linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org; >> stable@vger.kernel.org; devel@acpica.org >> Subject: Re: [Devel] [PATCH] ACPICA: cleanup method properly on error >> >> We'll look at this. >> Thanks. >> >> >>> -----Original Message----- >>> From: Vegard Nossum [mailto:vegard.nossum@oracle.com] >>> Sent: Friday, July 22, 2016 8:35 AM >>> To: Moore, Robert ; Zheng, Lv >>> ; Wysocki, Rafael J >>> Cc: linux-acpi@vger.kernel.org; devel@acpica.org; linux- >>> kernel@vger.kernel.org; Vegard Nossum ; >>> stable@vger.kernel.org >>> Subject: [PATCH] ACPICA: cleanup method properly on error >>> >>> If the call to acpi_ds_init_aml_walk() fails, then we have to undo the >>> walk state push done by acpi_ds_create_walk_state(). Otherwise, the >>> new walk state (which has just been freed) will remain on the thread's >>> walk_state_list and be dereferenced in acpi_ps_parse_aml() when we try >>> to get the new state. >>> >>> You can observe this when reading e.g. >>> >>> /sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0F:01/status >>> >>> Cc: stable@vger.kernel.org >>> Signed-off-by: Vegard Nossum >>> --- >>> drivers/acpi/acpica/dsmethod.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/drivers/acpi/acpica/dsmethod.c >>> b/drivers/acpi/acpica/dsmethod.c index 47c7b52..44b50a6 100644 >>> --- a/drivers/acpi/acpica/dsmethod.c >>> +++ b/drivers/acpi/acpica/dsmethod.c >>> @@ -596,6 +596,8 @@ cleanup: >>> /* On error, we must terminate the method properly */ >>> >>> acpi_ds_terminate_control_method(obj_desc, next_walk_state); >>> + if (thread) >>> + acpi_ds_pop_walk_state(thread); >>> acpi_ds_delete_walk_state(next_walk_state); >>> >>> return_ACPI_STATUS(status); >>> -- >>> 1.9.1 >> >> _______________________________________________ >> Devel mailing list >> Devel@acpica.org >> https://lists.acpica.org/mailman/listinfo/devel