Patchwork acpi: cstates: set state->present[nr] if nr < MAX_CSTATE

login
register
mail settings
Submitter Colin King
Date May 7, 2013, 12:13 a.m.
Message ID <1367885613-23546-1-git-send-email-colin.king@canonical.com>
Download mbox | patch
Permalink /patch/241856/
State Accepted
Headers show

Comments

Colin King - May 7, 2013, 12:13 a.m.
From: Colin Ian King <colin.king@canonical.com>

Coverity CID #997274, Out-of-bounds write (OVERRUN)

We can get an out of bounds write on state->present[] if
nr >= MAX_CSTATE, so guard against this.  We already do this
for state->counts[].

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/acpi/cstates/cstates.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Alex Hung - May 10, 2013, 1:07 a.m.
On 05/07/2013 08:13 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Coverity CID #997274, Out-of-bounds write (OVERRUN)
>
> We can get an out of bounds write on state->present[] if
> nr >= MAX_CSTATE, so guard against this.  We already do this
> for state->counts[].
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/acpi/cstates/cstates.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/acpi/cstates/cstates.c b/src/acpi/cstates/cstates.c
> index 93a7e50..90b5c03 100644
> --- a/src/acpi/cstates/cstates.c
> +++ b/src/acpi/cstates/cstates.c
> @@ -100,10 +100,10 @@ static void get_cstates(char *path, fwts_cstates *state)
>   			count = strtoull(data, NULL, 10);
>   			free(data);
>
> -			if ((nr>=0) && (nr < MAX_CSTATE))
> +			if ((nr >= 0) && (nr < MAX_CSTATE)) {
>   				state->counts[nr] = count;
> -
> -			state->present[nr] = true;
> +				state->present[nr] = true;
> +			}
>   		}
>   	}
>   	closedir(dir);
>
Acked-by: Alex Hung <alex.hung@canonical.com>
Ivan Hu - May 16, 2013, 4:02 a.m.
On 05/07/2013 08:13 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Coverity CID #997274, Out-of-bounds write (OVERRUN)
>
> We can get an out of bounds write on state->present[] if
> nr >= MAX_CSTATE, so guard against this.  We already do this
> for state->counts[].
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/acpi/cstates/cstates.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/acpi/cstates/cstates.c b/src/acpi/cstates/cstates.c
> index 93a7e50..90b5c03 100644
> --- a/src/acpi/cstates/cstates.c
> +++ b/src/acpi/cstates/cstates.c
> @@ -100,10 +100,10 @@ static void get_cstates(char *path, fwts_cstates *state)
>   			count = strtoull(data, NULL, 10);
>   			free(data);
>
> -			if ((nr>=0) && (nr < MAX_CSTATE))
> +			if ((nr >= 0) && (nr < MAX_CSTATE)) {
>   				state->counts[nr] = count;
> -
> -			state->present[nr] = true;
> +				state->present[nr] = true;
> +			}
>   		}
>   	}
>   	closedir(dir);
>

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

Patch

diff --git a/src/acpi/cstates/cstates.c b/src/acpi/cstates/cstates.c
index 93a7e50..90b5c03 100644
--- a/src/acpi/cstates/cstates.c
+++ b/src/acpi/cstates/cstates.c
@@ -100,10 +100,10 @@  static void get_cstates(char *path, fwts_cstates *state)
 			count = strtoull(data, NULL, 10);
 			free(data);
 
-			if ((nr>=0) && (nr < MAX_CSTATE))
+			if ((nr >= 0) && (nr < MAX_CSTATE)) {
 				state->counts[nr] = count;
-
-			state->present[nr] = true;
+				state->present[nr] = true;
+			}
 		}
 	}
 	closedir(dir);