[PATCH] lib/fwts_uefi.c: fix time-of-use race on open/stat (LP: #1209251)

[Keng-Yu Lin]

On Wed, Aug 7, 2013 at 10:12 PM, Colin King <colin.king at canonical.com> wrote:
> From: Colin Ian King <colin.king at canonical.com>
>
> Coverity Scan reports that the stat/open may give a small window
> of opportunity for a stat/open race condition. Instead, do open
> and fstat.
>
> Signed-off-by: Colin Ian King <colin.king at canonical.com>
> ---
>  src/lib/src/fwts_uefi.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/src/lib/src/fwts_uefi.c b/src/lib/src/fwts_uefi.c
> index 55308ba..f3576f2 100644
> --- a/src/lib/src/fwts_uefi.c
> +++ b/src/lib/src/fwts_uefi.c
> @@ -270,33 +270,35 @@ static int fwts_uefi_get_variable_efivars_fs(const char *varname, fwts_uefi_var
>
>         snprintf(filename, sizeof(filename), "%s/%s", path, varname);
>
> -       if (stat(filename, &statbuf) < 0)
> +       if ((fd = open(filename, O_RDONLY)) < 0)
> +               return FWTS_ERROR;
> +
> +       if (fstat(fd, &statbuf) < 0) {
> +               close(fd);
>                 return FWTS_ERROR;
> +       }
>
>         /* Variable name, less the GUID, in 16 bit ints */
>         var->varname = calloc(1, (varname_len + 1 - 36)  * sizeof(uint16_t));
> -       if (var->varname == NULL)
> +       if (var->varname == NULL) {
> +               close(fd);
>                 return FWTS_ERROR;
> +       }
>
>         /* Convert name to internal 16 bit version */
>         fwts_uefi_str_to_str16(var->varname, varname_len - 36, varname);
>
>         /* Need to read the data in one read, so allocate a buffer big enough */
>         if ((efivars_fs_var = calloc(1, statbuf.st_size)) == NULL) {
> +               close(fd);
>                 free(var->varname);
>                 return FWTS_ERROR;
>         }
>
> -       if ((fd = open(filename, O_RDONLY)) < 0) {
> -               free(var->varname);
> -               free(efivars_fs_var);
> -               return FWTS_ERROR;
> -       }
> -
>         if (read(fd, efivars_fs_var, statbuf.st_size) != statbuf.st_size) {
> +               close(fd);
>                 free(var->varname);
>                 free(efivars_fs_var);
> -               close(fd);
>                 return FWTS_ERROR;
>         }
>         close(fd);
> --
> 1.8.3.2
>
Acked-by: Keng-Yu Lin <kengyu at canonical.com>>