Patchwork lib: fix assembly/disassembly segfault when AcpiOsVprintf is called (LP: #1195209)

login
register
mail settings
Submitter Colin King
Date June 27, 2013, 10:25 a.m.
Message ID <1372328758-21583-1-git-send-email-colin.king@canonical.com>
Download mbox | patch
Permalink /patch/255033/
State Accepted
Headers show

Comments

Colin King - June 27, 2013, 10:25 a.m.
From: Colin Ian King <colin.king@canonical.com>

fwts --dis or fwts syntaxcheck can segfault if the ACPICA disassemble emits a
message via AcpiOsVprintf(). This calls the fwts_acpica helper function that
attempts to print to the log file which fails because the fw pointer is null.
This bug is a regression introduced with commit
54f1ef23a1c3bcbea023ddc799dc7d6becf60556.

To fix this, add a helper function to fwts_acpica that sets up the fwts
framework pointer so that fwts_acpica_vprintf won't segfault. Also sanity
check the framework pointer in fwts_acpica_vprintf in case it isn't set up
correctly.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/acpica/fwts_acpica.c      | 13 ++++++++++++-
 src/lib/include/fwts_acpica.h |  1 +
 src/lib/src/fwts_iasl.c       |  5 +++++
 3 files changed, 18 insertions(+), 1 deletion(-)
Keng-Yu Lin - June 28, 2013, 2:28 a.m.
On Thu, Jun 27, 2013 at 6:25 PM, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> fwts --dis or fwts syntaxcheck can segfault if the ACPICA disassemble emits a
> message via AcpiOsVprintf(). This calls the fwts_acpica helper function that
> attempts to print to the log file which fails because the fw pointer is null.
> This bug is a regression introduced with commit
> 54f1ef23a1c3bcbea023ddc799dc7d6becf60556.
>
> To fix this, add a helper function to fwts_acpica that sets up the fwts
> framework pointer so that fwts_acpica_vprintf won't segfault. Also sanity
> check the framework pointer in fwts_acpica_vprintf in case it isn't set up
> correctly.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  src/acpica/fwts_acpica.c      | 13 ++++++++++++-
>  src/lib/include/fwts_acpica.h |  1 +
>  src/lib/src/fwts_iasl.c       |  5 +++++
>  3 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/src/acpica/fwts_acpica.c b/src/acpica/fwts_acpica.c
> index e396cd9..a0b551e 100644
> --- a/src/acpica/fwts_acpica.c
> +++ b/src/acpica/fwts_acpica.c
> @@ -396,6 +396,9 @@ void fwts_acpica_vprintf(const char *fmt, va_list args)
>         char *tmp;
>         size_t tmp_len;
>
> +       if (fwts_acpica_fw == NULL)
> +               return;
> +
>         /* Only emit messages if in ACPICA debug mode */
>         if (!(fwts_acpica_fw->flags & FWTS_FLAG_ACPICA_DEBUG))
>                 return;
> @@ -864,6 +867,14 @@ int fwtsInstallEarlyHandlers(fwts_framework *fw)
>         return FWTS_OK;
>  }
>
> +/*
> + *  fwts_acpcia_set_fwts_framework()
> + *     set fwts_acpica_fw ptr
> + */
> +void fwts_acpcia_set_fwts_framework(fwts_framework *fw)
> +{
> +       fwts_acpica_fw = fw;
> +}
>
>  /*
>   *  fwts_acpica_init()
> @@ -883,7 +894,7 @@ int fwts_acpica_init(fwts_framework *fw)
>         pthread_mutex_init(&mutex_lock_sem_table, NULL);
>         pthread_mutex_init(&mutex_thread_info, NULL);
>
> -       fwts_acpica_fw = fw;
> +       fwts_acpcia_set_fwts_framework(fw);
>
>         AcpiDbgLevel = ACPI_NORMAL_DEFAULT;
>         AcpiDbgLayer = 0x00000000;
> diff --git a/src/lib/include/fwts_acpica.h b/src/lib/include/fwts_acpica.h
> index ab492e7..59bc6ef 100644
> --- a/src/lib/include/fwts_acpica.h
> +++ b/src/lib/include/fwts_acpica.h
> @@ -24,6 +24,7 @@
>
>  typedef void (*fwts_acpica_log_callback)(fwts_framework *fw, const char *buffer);
>
> +void fwts_acpcia_set_fwts_framework(fwts_framework *fw);
>  int  fwts_acpica_init(fwts_framework *fw);
>  int  fwts_acpica_deinit(void);
>  fwts_list *fwts_acpica_get_object_names(int type);
> diff --git a/src/lib/src/fwts_iasl.c b/src/lib/src/fwts_iasl.c
> index 6877c34..5058c1e 100644
> --- a/src/lib/src/fwts_iasl.c
> +++ b/src/lib/src/fwts_iasl.c
> @@ -29,6 +29,7 @@
>
>  #include "fwts.h"
>  #include "fwts_iasl_interface.h"
> +#include "fwts_acpica.h"
>
>  /*
>   *  fwts_iasl_dump_aml_to_file()
> @@ -75,6 +76,8 @@ int fwts_iasl_disassemble_to_file(fwts_framework *fw,
>         int pid = getpid();
>         int ret;
>
> +       fwts_acpcia_set_fwts_framework(fw);
> +
>         if ((ret = fwts_acpi_find_table(fw, tablename, which, &table)) != FWTS_OK)
>                 return ret;
>
> @@ -179,6 +182,8 @@ int fwts_iasl_reassemble(fwts_framework *fw,
>         if ((iasl_disassembly  == NULL) || (iasl_errors == NULL))
>                 return FWTS_ERROR;
>
> +       fwts_acpcia_set_fwts_framework(fw);
> +
>         *iasl_disassembly = NULL;
>
>         snprintf(tmpfile, sizeof(tmpfile), "/tmp/fwts_iasl_%d.dsl", pid);
> --
> 1.8.3.1
>

Acked-by: Keng-Yu Lin <kengyu@canonical.com>
Alex Hung - June 28, 2013, 3:35 a.m.
On 06/27/2013 06:25 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> fwts --dis or fwts syntaxcheck can segfault if the ACPICA disassemble emits a
> message via AcpiOsVprintf(). This calls the fwts_acpica helper function that
> attempts to print to the log file which fails because the fw pointer is null.
> This bug is a regression introduced with commit
> 54f1ef23a1c3bcbea023ddc799dc7d6becf60556.
> 
> To fix this, add a helper function to fwts_acpica that sets up the fwts
> framework pointer so that fwts_acpica_vprintf won't segfault. Also sanity
> check the framework pointer in fwts_acpica_vprintf in case it isn't set up
> correctly.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  src/acpica/fwts_acpica.c      | 13 ++++++++++++-
>  src/lib/include/fwts_acpica.h |  1 +
>  src/lib/src/fwts_iasl.c       |  5 +++++
>  3 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/src/acpica/fwts_acpica.c b/src/acpica/fwts_acpica.c
> index e396cd9..a0b551e 100644
> --- a/src/acpica/fwts_acpica.c
> +++ b/src/acpica/fwts_acpica.c
> @@ -396,6 +396,9 @@ void fwts_acpica_vprintf(const char *fmt, va_list args)
>  	char *tmp;
>  	size_t tmp_len;
>  
> +	if (fwts_acpica_fw == NULL)
> +		return;
> +
>  	/* Only emit messages if in ACPICA debug mode */
>  	if (!(fwts_acpica_fw->flags & FWTS_FLAG_ACPICA_DEBUG))
>  		return;
> @@ -864,6 +867,14 @@ int fwtsInstallEarlyHandlers(fwts_framework *fw)
>  	return FWTS_OK;
>  }
>  
> +/*
> + *  fwts_acpcia_set_fwts_framework()
> + *	set fwts_acpica_fw ptr
> + */
> +void fwts_acpcia_set_fwts_framework(fwts_framework *fw)
> +{
> +	fwts_acpica_fw = fw;
> +}
>  
>  /*
>   *  fwts_acpica_init()
> @@ -883,7 +894,7 @@ int fwts_acpica_init(fwts_framework *fw)
>  	pthread_mutex_init(&mutex_lock_sem_table, NULL);
>  	pthread_mutex_init(&mutex_thread_info, NULL);
>  
> -	fwts_acpica_fw = fw;
> +	fwts_acpcia_set_fwts_framework(fw);
>  
>  	AcpiDbgLevel = ACPI_NORMAL_DEFAULT;
>  	AcpiDbgLayer = 0x00000000;
> diff --git a/src/lib/include/fwts_acpica.h b/src/lib/include/fwts_acpica.h
> index ab492e7..59bc6ef 100644
> --- a/src/lib/include/fwts_acpica.h
> +++ b/src/lib/include/fwts_acpica.h
> @@ -24,6 +24,7 @@
>  
>  typedef void (*fwts_acpica_log_callback)(fwts_framework *fw, const char *buffer);
>  
> +void fwts_acpcia_set_fwts_framework(fwts_framework *fw);
>  int  fwts_acpica_init(fwts_framework *fw);
>  int  fwts_acpica_deinit(void);
>  fwts_list *fwts_acpica_get_object_names(int type);
> diff --git a/src/lib/src/fwts_iasl.c b/src/lib/src/fwts_iasl.c
> index 6877c34..5058c1e 100644
> --- a/src/lib/src/fwts_iasl.c
> +++ b/src/lib/src/fwts_iasl.c
> @@ -29,6 +29,7 @@
>  
>  #include "fwts.h"
>  #include "fwts_iasl_interface.h"
> +#include "fwts_acpica.h"
>  
>  /*
>   *  fwts_iasl_dump_aml_to_file()
> @@ -75,6 +76,8 @@ int fwts_iasl_disassemble_to_file(fwts_framework *fw,
>  	int pid = getpid();
>  	int ret;
>  
> +	fwts_acpcia_set_fwts_framework(fw);
> +
>  	if ((ret = fwts_acpi_find_table(fw, tablename, which, &table)) != FWTS_OK)
>  		return ret;
>  
> @@ -179,6 +182,8 @@ int fwts_iasl_reassemble(fwts_framework *fw,
>  	if ((iasl_disassembly  == NULL) || (iasl_errors == NULL))
>  		return FWTS_ERROR;
>  
> +	fwts_acpcia_set_fwts_framework(fw);
> +
>  	*iasl_disassembly = NULL;
>  
>  	snprintf(tmpfile, sizeof(tmpfile), "/tmp/fwts_iasl_%d.dsl", pid);
> 
Acked-by: Alex Hung <alex.hung@canonical.com>

Patch

diff --git a/src/acpica/fwts_acpica.c b/src/acpica/fwts_acpica.c
index e396cd9..a0b551e 100644
--- a/src/acpica/fwts_acpica.c
+++ b/src/acpica/fwts_acpica.c
@@ -396,6 +396,9 @@  void fwts_acpica_vprintf(const char *fmt, va_list args)
 	char *tmp;
 	size_t tmp_len;
 
+	if (fwts_acpica_fw == NULL)
+		return;
+
 	/* Only emit messages if in ACPICA debug mode */
 	if (!(fwts_acpica_fw->flags & FWTS_FLAG_ACPICA_DEBUG))
 		return;
@@ -864,6 +867,14 @@  int fwtsInstallEarlyHandlers(fwts_framework *fw)
 	return FWTS_OK;
 }
 
+/*
+ *  fwts_acpcia_set_fwts_framework()
+ *	set fwts_acpica_fw ptr
+ */
+void fwts_acpcia_set_fwts_framework(fwts_framework *fw)
+{
+	fwts_acpica_fw = fw;
+}
 
 /*
  *  fwts_acpica_init()
@@ -883,7 +894,7 @@  int fwts_acpica_init(fwts_framework *fw)
 	pthread_mutex_init(&mutex_lock_sem_table, NULL);
 	pthread_mutex_init(&mutex_thread_info, NULL);
 
-	fwts_acpica_fw = fw;
+	fwts_acpcia_set_fwts_framework(fw);
 
 	AcpiDbgLevel = ACPI_NORMAL_DEFAULT;
 	AcpiDbgLayer = 0x00000000;
diff --git a/src/lib/include/fwts_acpica.h b/src/lib/include/fwts_acpica.h
index ab492e7..59bc6ef 100644
--- a/src/lib/include/fwts_acpica.h
+++ b/src/lib/include/fwts_acpica.h
@@ -24,6 +24,7 @@ 
 
 typedef void (*fwts_acpica_log_callback)(fwts_framework *fw, const char *buffer);
 
+void fwts_acpcia_set_fwts_framework(fwts_framework *fw);
 int  fwts_acpica_init(fwts_framework *fw);
 int  fwts_acpica_deinit(void);
 fwts_list *fwts_acpica_get_object_names(int type);
diff --git a/src/lib/src/fwts_iasl.c b/src/lib/src/fwts_iasl.c
index 6877c34..5058c1e 100644
--- a/src/lib/src/fwts_iasl.c
+++ b/src/lib/src/fwts_iasl.c
@@ -29,6 +29,7 @@ 
 
 #include "fwts.h"
 #include "fwts_iasl_interface.h"
+#include "fwts_acpica.h"
 
 /*
  *  fwts_iasl_dump_aml_to_file()
@@ -75,6 +76,8 @@  int fwts_iasl_disassemble_to_file(fwts_framework *fw,
 	int pid = getpid();
 	int ret;
 
+	fwts_acpcia_set_fwts_framework(fw);
+
 	if ((ret = fwts_acpi_find_table(fw, tablename, which, &table)) != FWTS_OK)
 		return ret;
 
@@ -179,6 +182,8 @@  int fwts_iasl_reassemble(fwts_framework *fw,
 	if ((iasl_disassembly  == NULL) || (iasl_errors == NULL))
 		return FWTS_ERROR;
 
+	fwts_acpcia_set_fwts_framework(fw);
+
 	*iasl_disassembly = NULL;
 
 	snprintf(tmpfile, sizeof(tmpfile), "/tmp/fwts_iasl_%d.dsl", pid);