* [PATCH 1/2] ACPI / ACPICA: refactor acpi_evaluate_object() to reduce nesting if
@ 2014-01-24 16:20 Hanjun Guo
2014-01-24 16:20 ` [PATCH 2/2] ACPI / ACPICA: Fix potential memory leak in acpi_evaluate_object() Hanjun Guo
2014-01-24 16:29 ` [PATCH 1/2] ACPI / ACPICA: refactor acpi_evaluate_object() to reduce nesting if Moore, Robert
0 siblings, 2 replies; 3+ messages in thread
From: Hanjun Guo @ 2014-01-24 16:20 UTC (permalink / raw)
To: Lv Zheng, Robert Moore
Cc: Rafael J. Wysocki, linux-acpi, devel, patches, linux-kernel,
linaro-acpi, Hanjun Guo
acpi_evaluate_object() has nesting if judgement to make code a little
bit complicated, refactor it to make things simple.
Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
---
drivers/acpi/acpica/nsxfeval.c | 104 +++++++++++++++++++---------------------
1 file changed, 50 insertions(+), 54 deletions(-)
diff --git a/drivers/acpi/acpica/nsxfeval.c b/drivers/acpi/acpica/nsxfeval.c
index 1f0c28b..a1b0b88 100644
--- a/drivers/acpi/acpica/nsxfeval.c
+++ b/drivers/acpi/acpica/nsxfeval.c
@@ -370,68 +370,64 @@ acpi_evaluate_object(acpi_handle handle,
* If we are expecting a return value, and all went well above,
* copy the return value to an external object.
*/
- if (return_buffer) {
- if (!info->return_object) {
- return_buffer->length = 0;
- } else {
- if (ACPI_GET_DESCRIPTOR_TYPE(info->return_object) ==
- ACPI_DESC_TYPE_NAMED) {
- /*
- * If we received a NS Node as a return object, this means that
- * the object we are evaluating has nothing interesting to
- * return (such as a mutex, etc.) We return an error because
- * these types are essentially unsupported by this interface.
- * We don't check up front because this makes it easier to add
- * support for various types at a later date if necessary.
- */
- status = AE_TYPE;
- info->return_object = NULL; /* No need to delete a NS Node */
- return_buffer->length = 0;
- }
+ if (!return_buffer)
+ goto out;
- if (ACPI_SUCCESS(status)) {
+ if (!info->return_object) {
+ return_buffer->length = 0;
+ goto out;
+ }
- /* Dereference Index and ref_of references */
+ if (ACPI_GET_DESCRIPTOR_TYPE(info->return_object) ==
+ ACPI_DESC_TYPE_NAMED) {
+ /*
+ * If we received a NS Node as a return object, this means that
+ * the object we are evaluating has nothing interesting to
+ * return (such as a mutex, etc.) We return an error because
+ * these types are essentially unsupported by this interface.
+ * We don't check up front because this makes it easier to add
+ * support for various types at a later date if necessary.
+ */
+ status = AE_TYPE;
+ info->return_object = NULL; /* No need to delete a NS Node */
+ return_buffer->length = 0;
+ }
- acpi_ns_resolve_references(info);
+ if (ACPI_FAILURE(status))
+ goto out;
- /* Get the size of the returned object */
+ /* Dereference Index and ref_of references */
- status =
- acpi_ut_get_object_size(info->return_object,
- &buffer_space_needed);
- if (ACPI_SUCCESS(status)) {
-
- /* Validate/Allocate/Clear caller buffer */
-
- status =
- acpi_ut_initialize_buffer
- (return_buffer,
- buffer_space_needed);
- if (ACPI_FAILURE(status)) {
- /*
- * Caller's buffer is too small or a new one can't
- * be allocated
- */
- ACPI_DEBUG_PRINT((ACPI_DB_INFO,
- "Needed buffer size %X, %s\n",
- (u32)
- buffer_space_needed,
- acpi_format_exception
- (status)));
- } else {
- /* We have enough space for the object, build it */
-
- status =
- acpi_ut_copy_iobject_to_eobject
- (info->return_object,
- return_buffer);
- }
- }
- }
+ acpi_ns_resolve_references(info);
+
+ /* Get the size of the returned object */
+
+ status = acpi_ut_get_object_size(info->return_object,
+ &buffer_space_needed);
+ if (ACPI_SUCCESS(status)) {
+
+ /* Validate/Allocate/Clear caller buffer */
+
+ status = acpi_ut_initialize_buffer(return_buffer,
+ buffer_space_needed);
+ if (ACPI_FAILURE(status)) {
+ /*
+ * Caller's buffer is too small or a new one can't
+ * be allocated
+ */
+ ACPI_DEBUG_PRINT((ACPI_DB_INFO,
+ "Needed buffer size %X, %s\n",
+ (u32)buffer_space_needed,
+ acpi_format_exception(status)));
+ } else {
+ /* We have enough space for the object, build it */
+
+ status = acpi_ut_copy_iobject_to_eobject(
+ info->return_object, return_buffer);
}
}
+out:
if (info->return_object) {
/*
* Delete the internal return object. NOTE: Interpreter must be
--
1.7.9.5
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH 2/2] ACPI / ACPICA: Fix potential memory leak in acpi_evaluate_object()
2014-01-24 16:20 [PATCH 1/2] ACPI / ACPICA: refactor acpi_evaluate_object() to reduce nesting if Hanjun Guo
@ 2014-01-24 16:20 ` Hanjun Guo
2014-01-24 16:29 ` [PATCH 1/2] ACPI / ACPICA: refactor acpi_evaluate_object() to reduce nesting if Moore, Robert
1 sibling, 0 replies; 3+ messages in thread
From: Hanjun Guo @ 2014-01-24 16:20 UTC (permalink / raw)
To: Lv Zheng, Robert Moore
Cc: Rafael J. Wysocki, linux-acpi, devel, patches, linux-kernel,
linaro-acpi, Hanjun Guo, Yijing Wang
There is a potential memory leak when acpi_ut_copy_iobject_to_eobject()
failed, because return_buffer was allocated in acpi_ut_initialize_buffer()
when buffer type is ACPI_ALLOCATE_BUFFER or ACPI_ALLOCATE_LOCAL_BUFFER,
and will not be freed outside when the return value is not AE_OK for
acpi_evaluate_object(), fix it.
Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
drivers/acpi/acpica/nsxfeval.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/acpi/acpica/nsxfeval.c b/drivers/acpi/acpica/nsxfeval.c
index a1b0b88..8ad792b 100644
--- a/drivers/acpi/acpica/nsxfeval.c
+++ b/drivers/acpi/acpica/nsxfeval.c
@@ -424,6 +424,11 @@ acpi_evaluate_object(acpi_handle handle,
status = acpi_ut_copy_iobject_to_eobject(
info->return_object, return_buffer);
+ if (ACPI_FAILURE(status) &&
+ (buffer_space_needed == ACPI_ALLOCATE_BUFFER ||
+ buffer_space_needed == ACPI_ALLOCATE_LOCAL_BUFFER)) {
+ ACPI_FREE(return_buffer);
+ }
}
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 3+ messages in thread
* RE: [PATCH 1/2] ACPI / ACPICA: refactor acpi_evaluate_object() to reduce nesting if
2014-01-24 16:20 [PATCH 1/2] ACPI / ACPICA: refactor acpi_evaluate_object() to reduce nesting if Hanjun Guo
2014-01-24 16:20 ` [PATCH 2/2] ACPI / ACPICA: Fix potential memory leak in acpi_evaluate_object() Hanjun Guo
@ 2014-01-24 16:29 ` Moore, Robert
1 sibling, 0 replies; 3+ messages in thread
From: Moore, Robert @ 2014-01-24 16:29 UTC (permalink / raw)
To: Hanjun Guo, Zheng, Lv
Cc: Rafael J. Wysocki, linux-acpi, devel, patches, linux-kernel, linaro-acpi
Please file a bugzilla on this at:
https://bugs.acpica.org/
thanks.
Bob
> -----Original Message-----
> From: Hanjun Guo [mailto:hanjun.guo@linaro.org]
> Sent: Friday, January 24, 2014 8:20 AM
> To: Zheng, Lv; Moore, Robert
> Cc: Rafael J. Wysocki; linux-acpi@vger.kernel.org; devel@acpica.org;
> patches@linaro.org; linux-kernel@vger.kernel.org; linaro-
> acpi@lists.linaro.org; Hanjun Guo
> Subject: [PATCH 1/2] ACPI / ACPICA: refactor acpi_evaluate_object() to
> reduce nesting if
>
> acpi_evaluate_object() has nesting if judgement to make code a little bit
> complicated, refactor it to make things simple.
>
> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> ---
> drivers/acpi/acpica/nsxfeval.c | 104 +++++++++++++++++++----------------
> -----
> 1 file changed, 50 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/acpi/acpica/nsxfeval.c
> b/drivers/acpi/acpica/nsxfeval.c index 1f0c28b..a1b0b88 100644
> --- a/drivers/acpi/acpica/nsxfeval.c
> +++ b/drivers/acpi/acpica/nsxfeval.c
> @@ -370,68 +370,64 @@ acpi_evaluate_object(acpi_handle handle,
> * If we are expecting a return value, and all went well above,
> * copy the return value to an external object.
> */
> - if (return_buffer) {
> - if (!info->return_object) {
> - return_buffer->length = 0;
> - } else {
> - if (ACPI_GET_DESCRIPTOR_TYPE(info->return_object) ==
> - ACPI_DESC_TYPE_NAMED) {
> - /*
> - * If we received a NS Node as a return object,
> this means that
> - * the object we are evaluating has nothing
> interesting to
> - * return (such as a mutex, etc.) We return an
> error because
> - * these types are essentially unsupported by this
> interface.
> - * We don't check up front because this makes it
> easier to add
> - * support for various types at a later date if
> necessary.
> - */
> - status = AE_TYPE;
> - info->return_object = NULL; /* No need to delete
> a NS Node */
> - return_buffer->length = 0;
> - }
> + if (!return_buffer)
> + goto out;
>
> - if (ACPI_SUCCESS(status)) {
> + if (!info->return_object) {
> + return_buffer->length = 0;
> + goto out;
> + }
>
> - /* Dereference Index and ref_of references */
> + if (ACPI_GET_DESCRIPTOR_TYPE(info->return_object) ==
> + ACPI_DESC_TYPE_NAMED) {
> + /*
> + * If we received a NS Node as a return object, this means
> that
> + * the object we are evaluating has nothing interesting to
> + * return (such as a mutex, etc.) We return an error because
> + * these types are essentially unsupported by this interface.
> + * We don't check up front because this makes it easier to add
> + * support for various types at a later date if necessary.
> + */
> + status = AE_TYPE;
> + info->return_object = NULL; /* No need to delete a NS Node
> */
> + return_buffer->length = 0;
> + }
>
> - acpi_ns_resolve_references(info);
> + if (ACPI_FAILURE(status))
> + goto out;
>
> - /* Get the size of the returned object */
> + /* Dereference Index and ref_of references */
>
> - status =
> - acpi_ut_get_object_size(info->return_object,
> - &buffer_space_needed);
> - if (ACPI_SUCCESS(status)) {
> -
> - /* Validate/Allocate/Clear caller buffer */
> -
> - status =
> - acpi_ut_initialize_buffer
> - (return_buffer,
> - buffer_space_needed);
> - if (ACPI_FAILURE(status)) {
> - /*
> - * Caller's buffer is too small or a
> new one can't
> - * be allocated
> - */
> - ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> - "Needed buffer size %X,
> %s\n",
> - (u32)
> - buffer_space_needed,
> - acpi_format_exception
> - (status)));
> - } else {
> - /* We have enough space for the
> object, build it */
> -
> - status =
> - acpi_ut_copy_iobject_to_eobject
> - (info->return_object,
> - return_buffer);
> - }
> - }
> - }
> + acpi_ns_resolve_references(info);
> +
> + /* Get the size of the returned object */
> +
> + status = acpi_ut_get_object_size(info->return_object,
> + &buffer_space_needed);
> + if (ACPI_SUCCESS(status)) {
> +
> + /* Validate/Allocate/Clear caller buffer */
> +
> + status = acpi_ut_initialize_buffer(return_buffer,
> + buffer_space_needed);
> + if (ACPI_FAILURE(status)) {
> + /*
> + * Caller's buffer is too small or a new one can't
> + * be allocated
> + */
> + ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> + "Needed buffer size %X, %s\n",
> + (u32)buffer_space_needed,
> + acpi_format_exception(status)));
> + } else {
> + /* We have enough space for the object, build it */
> +
> + status = acpi_ut_copy_iobject_to_eobject(
> + info->return_object, return_buffer);
> }
> }
>
> +out:
> if (info->return_object) {
> /*
> * Delete the internal return object. NOTE: Interpreter must
> be
> --
> 1.7.9.5
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-01-24 16:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-24 16:20 [PATCH 1/2] ACPI / ACPICA: refactor acpi_evaluate_object() to reduce nesting if Hanjun Guo
2014-01-24 16:20 ` [PATCH 2/2] ACPI / ACPICA: Fix potential memory leak in acpi_evaluate_object() Hanjun Guo
2014-01-24 16:29 ` [PATCH 1/2] ACPI / ACPICA: refactor acpi_evaluate_object() to reduce nesting if Moore, Robert
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).