Patchwork lib: fwts_log_json: check sting from json is not null (LP: #1206285)

login
register
mail settings
Submitter Colin King
Date July 29, 2013, 10:05 p.m.
Message ID <1375135539-16798-1-git-send-email-colin.king@canonical.com>
Download mbox | patch
Permalink /patch/263065/
State Accepted
Headers show

Comments

Colin King - July 29, 2013, 10:05 p.m.
From: Colin Ian King <colin.king@canonical.com>

fwts_log_close_json() assumes json_object_to_string() always succeeds.
It may not and this will lead to a segfault, so check for null and
don't bail out with an exit but continue on because user may be using
other logging back-ends and some output is better than aborted output.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/lib/src/fwts_log_json.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)
Alex Hung - Aug. 1, 2013, 7:58 a.m.
On 07/30/2013 06:05 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> fwts_log_close_json() assumes json_object_to_string() always succeeds.
> It may not and this will lead to a segfault, so check for null and
> don't bail out with an exit but continue on because user may be using
> other logging back-ends and some output is better than aborted output.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/lib/src/fwts_log_json.c | 19 +++++++++++--------
>   1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/src/lib/src/fwts_log_json.c b/src/lib/src/fwts_log_json.c
> index bf28c0c..a67bc3a 100644
> --- a/src/lib/src/fwts_log_json.c
> +++ b/src/lib/src/fwts_log_json.c
> @@ -213,18 +213,21 @@ static void fwts_log_open_json(fwts_log_file *log_file)
>   static void fwts_log_close_json(fwts_log_file *log_file)
>   {
>   	const char *str;
> -	size_t len;
>
>   	fwts_log_section_end_json(log_file);
>
>   	str = json_object_to_json_string(json_stack[0].obj);
> -	len = strlen(str);
> -
> -	fwrite(str, 1, len, log_file->fp);
> -	fwrite("\n", 1, 1, log_file->fp);
> -	fflush(log_file->fp);
> -	log_file->line_number++;
> -
> +	if (str == NULL) {
> +		/* Let's not make this bail out as user may be logging to other files too */
> +		fprintf(stderr, "Cannot turn json object to text for output. Empty json output\n");
> +	} else {
> +		size_t len = strlen(str);
> +
> +		fwrite(str, 1, len, log_file->fp);
> +		fwrite("\n", 1, 1, log_file->fp);
> +		fflush(log_file->fp);
> +		log_file->line_number++;
> +	}
>   	json_object_put(json_stack[0].obj);
>   }
>
>

Acked-by: Alex Hung <alex.hung@canonical.com>
Ivan Hu - Aug. 2, 2013, 2:58 a.m.
On 07/30/2013 06:05 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> fwts_log_close_json() assumes json_object_to_string() always succeeds.
> It may not and this will lead to a segfault, so check for null and
> don't bail out with an exit but continue on because user may be using
> other logging back-ends and some output is better than aborted output.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/lib/src/fwts_log_json.c | 19 +++++++++++--------
>   1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/src/lib/src/fwts_log_json.c b/src/lib/src/fwts_log_json.c
> index bf28c0c..a67bc3a 100644
> --- a/src/lib/src/fwts_log_json.c
> +++ b/src/lib/src/fwts_log_json.c
> @@ -213,18 +213,21 @@ static void fwts_log_open_json(fwts_log_file *log_file)
>   static void fwts_log_close_json(fwts_log_file *log_file)
>   {
>   	const char *str;
> -	size_t len;
>
>   	fwts_log_section_end_json(log_file);
>
>   	str = json_object_to_json_string(json_stack[0].obj);
> -	len = strlen(str);
> -
> -	fwrite(str, 1, len, log_file->fp);
> -	fwrite("\n", 1, 1, log_file->fp);
> -	fflush(log_file->fp);
> -	log_file->line_number++;
> -
> +	if (str == NULL) {
> +		/* Let's not make this bail out as user may be logging to other files too */
> +		fprintf(stderr, "Cannot turn json object to text for output. Empty json output\n");
> +	} else {
> +		size_t len = strlen(str);
> +
> +		fwrite(str, 1, len, log_file->fp);
> +		fwrite("\n", 1, 1, log_file->fp);
> +		fflush(log_file->fp);
> +		log_file->line_number++;
> +	}
>   	json_object_put(json_stack[0].obj);
>   }
>
>
Acked-by: Ivan Hu <ivan.hu@canonical.com>

Patch

diff --git a/src/lib/src/fwts_log_json.c b/src/lib/src/fwts_log_json.c
index bf28c0c..a67bc3a 100644
--- a/src/lib/src/fwts_log_json.c
+++ b/src/lib/src/fwts_log_json.c
@@ -213,18 +213,21 @@  static void fwts_log_open_json(fwts_log_file *log_file)
 static void fwts_log_close_json(fwts_log_file *log_file)
 {
 	const char *str;
-	size_t len;
 
 	fwts_log_section_end_json(log_file);
 
 	str = json_object_to_json_string(json_stack[0].obj);
-	len = strlen(str);
-
-	fwrite(str, 1, len, log_file->fp);
-	fwrite("\n", 1, 1, log_file->fp);
-	fflush(log_file->fp);
-	log_file->line_number++;
-
+	if (str == NULL) {
+		/* Let's not make this bail out as user may be logging to other files too */
+		fprintf(stderr, "Cannot turn json object to text for output. Empty json output\n");
+	} else {
+		size_t len = strlen(str);
+
+		fwrite(str, 1, len, log_file->fp);
+		fwrite("\n", 1, 1, log_file->fp);
+		fflush(log_file->fp);
+		log_file->line_number++;
+	}
 	json_object_put(json_stack[0].obj);
 }