Patchwork lib: fwts_cpu.c: fix time-of-use race on stat/open (LP: #1209225)

login
register
mail settings
Submitter Colin King
Date Aug. 7, 2013, 12:42 p.m.
Message ID <1375879333-17487-1-git-send-email-colin.king@canonical.com>
Download mbox | patch
Permalink /patch/265505/
State Accepted
Headers show

Comments

Colin King - Aug. 7, 2013, 12:42 p.m.
From: Colin Ian King <colin.king@canonical.com>

Coverity Scan reports that the stat/open may give a small window
of opportunity for a stat/open race condition. So just forget the
stat as open will do.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/lib/src/fwts_cpu.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)
Keng-Yu Lin - Aug. 14, 2013, 3:30 a.m.
On Wed, Aug 7, 2013 at 8:42 PM, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Coverity Scan reports that the stat/open may give a small window
> of opportunity for a stat/open race condition. So just forget the
> stat as open will do.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  src/lib/src/fwts_cpu.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/src/lib/src/fwts_cpu.c b/src/lib/src/fwts_cpu.c
> index 44711fd..fc52fa5 100644
> --- a/src/lib/src/fwts_cpu.c
> +++ b/src/lib/src/fwts_cpu.c
> @@ -51,30 +51,23 @@ static pid_t *fwts_cpu_pids;
>   */
>  int fwts_cpu_readmsr(const int cpu, const uint32_t reg, uint64_t *val)
>  {
> -       struct stat statbuf;
>         char buffer[PATH_MAX];
> -       uint64_t value;
> +       uint64_t value = 0;
>         int fd;
>         int ret;
>
> -       value = 0;
> -
>         snprintf(buffer, sizeof(buffer), "/dev/cpu/%d/msr", cpu);
> -
> -       if (stat(buffer, &statbuf)) {
> +       if ((fd = open(buffer, O_RDONLY)) < 0) {
>                 /* Hrm, msr not there, so force modprobe msr and see what happens */
>                 pid_t pid;
>                 if ((fd = fwts_pipe_open("modprobe msr", &pid)) < 0)
>                         return FWTS_ERROR;
>                 fwts_pipe_close(fd, pid);
>
> -               if (stat(buffer, &statbuf))
> +               if ((fd = open(buffer, O_RDONLY)) < 0)
>                         return FWTS_ERROR; /* Really failed */
>         }
>
> -       if ((fd = open(buffer, O_RDONLY)) < 0)
> -                return FWTS_ERROR;
> -
>         ret = pread(fd, &value, 8, reg);
>         close(fd);
>
> --
> 1.8.3.2
>

Acked-by: Keng-Yu Lin <kengyu@canonical.com>
Alex Hung - Aug. 26, 2013, 3:55 a.m.
On 08/07/2013 08:42 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Coverity Scan reports that the stat/open may give a small window
> of opportunity for a stat/open race condition. So just forget the
> stat as open will do.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/lib/src/fwts_cpu.c | 13 +++----------
>   1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/src/lib/src/fwts_cpu.c b/src/lib/src/fwts_cpu.c
> index 44711fd..fc52fa5 100644
> --- a/src/lib/src/fwts_cpu.c
> +++ b/src/lib/src/fwts_cpu.c
> @@ -51,30 +51,23 @@ static pid_t *fwts_cpu_pids;
>    */
>   int fwts_cpu_readmsr(const int cpu, const uint32_t reg, uint64_t *val)
>   {
> -	struct stat statbuf;
>   	char buffer[PATH_MAX];
> -	uint64_t value;
> +	uint64_t value = 0;
>   	int fd;
>   	int ret;
>
> -	value = 0;
> -
>   	snprintf(buffer, sizeof(buffer), "/dev/cpu/%d/msr", cpu);
> -
> -	if (stat(buffer, &statbuf)) {
> +	if ((fd = open(buffer, O_RDONLY)) < 0) {
>   		/* Hrm, msr not there, so force modprobe msr and see what happens */
>   		pid_t pid;
>   		if ((fd = fwts_pipe_open("modprobe msr", &pid)) < 0)
>   			return FWTS_ERROR;
>   		fwts_pipe_close(fd, pid);
>
> -		if (stat(buffer, &statbuf))
> +		if ((fd = open(buffer, O_RDONLY)) < 0)
>   			return FWTS_ERROR; /* Really failed */
>   	}
>
> -	if ((fd = open(buffer, O_RDONLY)) < 0)
> -                return FWTS_ERROR;
> -
>   	ret = pread(fd, &value, 8, reg);
>   	close(fd);
>
>

Acked-by: Alex Hung <alex.hung@canonical.com>

Patch

diff --git a/src/lib/src/fwts_cpu.c b/src/lib/src/fwts_cpu.c
index 44711fd..fc52fa5 100644
--- a/src/lib/src/fwts_cpu.c
+++ b/src/lib/src/fwts_cpu.c
@@ -51,30 +51,23 @@  static pid_t *fwts_cpu_pids;
  */
 int fwts_cpu_readmsr(const int cpu, const uint32_t reg, uint64_t *val)
 {
-	struct stat statbuf;
 	char buffer[PATH_MAX];
-	uint64_t value;
+	uint64_t value = 0;
 	int fd;
 	int ret;
 
-	value = 0;
-
 	snprintf(buffer, sizeof(buffer), "/dev/cpu/%d/msr", cpu);
-
-	if (stat(buffer, &statbuf)) {
+	if ((fd = open(buffer, O_RDONLY)) < 0) {
 		/* Hrm, msr not there, so force modprobe msr and see what happens */
 		pid_t pid;
 		if ((fd = fwts_pipe_open("modprobe msr", &pid)) < 0)
 			return FWTS_ERROR;
 		fwts_pipe_close(fd, pid);
 
-		if (stat(buffer, &statbuf))
+		if ((fd = open(buffer, O_RDONLY)) < 0)
 			return FWTS_ERROR; /* Really failed */
 	}
 
-	if ((fd = open(buffer, O_RDONLY)) < 0)
-                return FWTS_ERROR;
-
 	ret = pread(fd, &value, 8, reg);
 	close(fd);