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

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

[Andrew Price]

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@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");
 }
-- 
1.8.3.1

[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@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");
>  }