diff mbox series

acpi: fan: fix potential null pointer dereference issue

Message ID 20180426082639.25299-1-colin.king@canonical.com
State Accepted
Headers show
Series acpi: fan: fix potential null pointer dereference issue | expand

Commit Message

Colin Ian King April 26, 2018, 8:26 a.m. UTC 
From: Colin Ian King <colin.king@canonical.com>

Check for non-null info->type before dereferencing it with a strcmp.

Detected by CoverityScan, CID#1390651 ("Null pointer dereferences")

Fixes: 0eb85f83c260 ("fan: cur_state == -1 is valid for intel_powerclamp")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/acpi/fan/fan.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

Comments

Alex Hung April 26, 2018, 6:37 p.m. UTC | #1 
On 2018-04-26 01:26 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> Check for non-null info->type before dereferencing it with a strcmp.
> 
> Detected by CoverityScan, CID#1390651 ("Null pointer dereferences")
> 
> Fixes: 0eb85f83c260 ("fan: cur_state == -1 is valid for intel_powerclamp")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/acpi/fan/fan.c | 23 +++++++++++++----------
>   1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/src/acpi/fan/fan.c b/src/acpi/fan/fan.c
> index 298d3a13..edd947e8 100644
> --- a/src/acpi/fan/fan.c
> +++ b/src/acpi/fan/fan.c
> @@ -154,16 +154,19 @@ static int fan_test1(fwts_framework *fw)
>   			fwts_failed(fw, LOG_LEVEL_MEDIUM, "NoFanMaxState",
>   				"Fan %s present but has no max_state present.",
>   				info->name);
> -		if (info->cur_state == -1 && strcmp(info->type, "intel_powerclamp"))
> -			fwts_failed(fw, LOG_LEVEL_MEDIUM, "NoFanCurState",
> -				"Fan %s present but has no cur_state present.",
> -				info->name);
> -
> -		if (info->type &&
> -		    (info->max_state >= 0) && (info->cur_state >= 0))
> -			fwts_passed(fw, "Fan %s of type %s has max cooling state %d "
> -				"and current cooling state %d.",
> -					info->name, info->type, info->max_state, info->cur_state);
> +		if (info->type) {
> +			if ((info->cur_state == -1) &&
> +			    strcmp(info->type, "intel_powerclamp"))
> +				fwts_failed(fw, LOG_LEVEL_MEDIUM, "NoFanCurState",
> +					"Fan %s present but has no cur_state present.",
> +					info->name);
> +
> +			if ((info->max_state >= 0) && (info->cur_state >= 0))
> +				fwts_passed(fw, "Fan %s of type %s has max cooling "
> +					"state %d and current cooling state %d.",
> +					info->name, info->type, info->max_state,
> +					info->cur_state);
> +		}
>   	}
>   
>   	fwts_list_free(fans, free_fan_info);
> 


Acked-by: Alex Hung <alex.hung@canonical.com>
Ivan Hu April 27, 2018, 7:28 a.m. UTC | #2 
On 04/26/2018 04:26 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Check for non-null info->type before dereferencing it with a strcmp.
>
> Detected by CoverityScan, CID#1390651 ("Null pointer dereferences")
>
> Fixes: 0eb85f83c260 ("fan: cur_state == -1 is valid for intel_powerclamp")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  src/acpi/fan/fan.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/src/acpi/fan/fan.c b/src/acpi/fan/fan.c
> index 298d3a13..edd947e8 100644
> --- a/src/acpi/fan/fan.c
> +++ b/src/acpi/fan/fan.c
> @@ -154,16 +154,19 @@ static int fan_test1(fwts_framework *fw)
>  			fwts_failed(fw, LOG_LEVEL_MEDIUM, "NoFanMaxState",
>  				"Fan %s present but has no max_state present.",
>  				info->name);
> -		if (info->cur_state == -1 && strcmp(info->type, "intel_powerclamp"))
> -			fwts_failed(fw, LOG_LEVEL_MEDIUM, "NoFanCurState",
> -				"Fan %s present but has no cur_state present.",
> -				info->name);
> -
> -		if (info->type &&
> -		    (info->max_state >= 0) && (info->cur_state >= 0))
> -			fwts_passed(fw, "Fan %s of type %s has max cooling state %d "
> -				"and current cooling state %d.",
> -					info->name, info->type, info->max_state, info->cur_state);
> +		if (info->type) {
> +			if ((info->cur_state == -1) &&
> +			    strcmp(info->type, "intel_powerclamp"))
> +				fwts_failed(fw, LOG_LEVEL_MEDIUM, "NoFanCurState",
> +					"Fan %s present but has no cur_state present.",
> +					info->name);
> +
> +			if ((info->max_state >= 0) && (info->cur_state >= 0))
> +				fwts_passed(fw, "Fan %s of type %s has max cooling "
> +					"state %d and current cooling state %d.",
> +					info->name, info->type, info->max_state,
> +					info->cur_state);
> +		}
>  	}
>  
>  	fwts_list_free(fans, free_fan_info);

Acked-by: Ivan Hu <ivan.hu@canonical.com>
diff mbox series

Patch

diff --git a/src/acpi/fan/fan.c b/src/acpi/fan/fan.c
index 298d3a13..edd947e8 100644
--- a/src/acpi/fan/fan.c
+++ b/src/acpi/fan/fan.c
@@ -154,16 +154,19 @@  static int fan_test1(fwts_framework *fw)
 			fwts_failed(fw, LOG_LEVEL_MEDIUM, "NoFanMaxState",
 				"Fan %s present but has no max_state present.",
 				info->name);
-		if (info->cur_state == -1 && strcmp(info->type, "intel_powerclamp"))
-			fwts_failed(fw, LOG_LEVEL_MEDIUM, "NoFanCurState",
-				"Fan %s present but has no cur_state present.",
-				info->name);
-
-		if (info->type &&
-		    (info->max_state >= 0) && (info->cur_state >= 0))
-			fwts_passed(fw, "Fan %s of type %s has max cooling state %d "
-				"and current cooling state %d.",
-					info->name, info->type, info->max_state, info->cur_state);
+		if (info->type) {
+			if ((info->cur_state == -1) &&
+			    strcmp(info->type, "intel_powerclamp"))
+				fwts_failed(fw, LOG_LEVEL_MEDIUM, "NoFanCurState",
+					"Fan %s present but has no cur_state present.",
+					info->name);
+
+			if ((info->max_state >= 0) && (info->cur_state >= 0))
+				fwts_passed(fw, "Fan %s of type %s has max cooling "
+					"state %d and current cooling state %d.",
+					info->name, info->type, info->max_state,
+					info->cur_state);
+		}
 	}
 
 	fwts_list_free(fans, free_fan_info);