[Cluster-devel] [PATCH RHEL6] libgfs2: Fix up remove_mtab_entry

[Steven Whitehouse]

Hi,

Looks good to me,

Steve.

On Tue, 2014-02-04 at 12:25 +0000, Andrew Price wrote:
> mkstemp was creating the temporary mtab file with overly tight
> permissions causing /etc/mtab to end up with an 0600 mode instead of the
> regular 0644. This patch resets the mode to 0644 by calling fchmod on
> the file descriptor returned by mkstemp.
> 
> It also removes a superfluous close() call and sets the temp filename
> char array on initialisation instead of using strcpy().
> 
> Resolves: rhbz#1059853
> 
> Signed-off-by: Andrew Price <anprice at redhat.com>
> ---
>  gfs2/libgfs2/misc.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/gfs2/libgfs2/misc.c b/gfs2/libgfs2/misc.c
> index 3989bf1..8e0ca6f 100644
> --- a/gfs2/libgfs2/misc.c
> +++ b/gfs2/libgfs2/misc.c
> @@ -217,7 +217,7 @@ static void remove_mtab_entry(struct gfs2_sbd *sdp)
>  {
>  	FILE *mtab, *mtabnew;
>  	struct mntent *mountent;
> -	char mtab_tmpfn[PATH_MAX];
> +	char mtab_tmpfn[] = "/etc/mtab.XXXXXX";
>  	int error, fd;
>  	mode_t mask;
>  
> @@ -225,7 +225,7 @@ static void remove_mtab_entry(struct gfs2_sbd *sdp)
>  	if (mtab == NULL)
>  		die("Couldn't open /etc/mtab for writing: %s\n",
>  		    strerror(errno));
> -	strcpy(mtab_tmpfn, "/etc/mtab.XXXXXX");
> +
>  	mask = umask(S_IXUSR | S_IRWXG | S_IRWXO);
>  	fd = mkstemp(mtab_tmpfn);
>  	umask(mask);
> @@ -233,6 +233,9 @@ static void remove_mtab_entry(struct gfs2_sbd *sdp)
>  		die("Couldn't open temporary mtab file for writing: %s\n",
>  		    strerror(errno));
>  
> +	if (fchmod(fd, S_IWUSR | S_IRUSR | S_IRGRP | S_IROTH))
> +		die("Couldn't change mode of temporary mtab: %s\n", strerror(errno));
> +
>  	mtabnew = fdopen(fd, "wt");
>  	if (mtabnew == NULL)
>  		die("Couldn't open %s for writing : %s\n", mtab_tmpfn,
> @@ -247,8 +250,7 @@ static void remove_mtab_entry(struct gfs2_sbd *sdp)
>  	}
>  
>  	endmntent(mtab);
> -	fclose(mtabnew);
> -	close(fd);
> +	fclose(mtabnew); /* Closes underlying fd */
>  	if (rename(mtab_tmpfn, "/etc/mtab"))
>  		fprintf(stderr, "Unable to remove mount entry from mtab.\n");
>  }