[Cluster-devel] [PATCH RHEL6] libgfs2: Use a matching context mount option in mount_gfs2_meta

[Cluster-devel] [PATCH RHEL6] libgfs2: Use a matching context mount option in mount_gfs2_meta

[Andrew Price]

On a system with SELinux enabled, if a gfs2 file system is mounted with
a context= option, the tools gfs2_quota, gfs2_tool, gfs2_grow and
gfs2_jadd will fail with "Device or resource busy". This is due to
SELinux failing the mount due to a mismatched context ("SELinux: mount
invalid.  Same superblock, different security settings").

In order to work around this, parse the context option of the gfs2 mount
point in is_pathname_mounted() and use it in mount_gfs2_meta().

Resolves: rhbz#1121693

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/libgfs2/libgfs2.h |  1 +
 gfs2/libgfs2/misc.c    | 21 ++++++++++++++++++++-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index 9c20f11..25286d1 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -217,6 +217,7 @@ struct gfs2_sbd {
 
 	int device_fd;
 	int path_fd;
+	char *secontext;
 
 	uint64_t sb_addr;
 
diff --git a/gfs2/libgfs2/misc.c b/gfs2/libgfs2/misc.c
index 8e0ca6f..5ef4a2a 100644
--- a/gfs2/libgfs2/misc.c
+++ b/gfs2/libgfs2/misc.c
@@ -100,6 +100,24 @@ int compute_constants(struct gfs2_sbd *sdp)
 	return 0;
 }
 
+/**
+ * Returns a duplicate of the 'context' mount option, or NULL if not found.
+ */
+static char *copy_context_opt(struct mntent *mnt)
+{
+	char *ctx, *end;
+
+	ctx = hasmntopt(mnt, "context");
+	if (ctx == NULL)
+		return NULL;
+
+	end = strchr(ctx, ',');
+	if (end == NULL)
+		return NULL;
+
+	return strndup(ctx, end - ctx);
+}
+
 int is_pathname_mounted(struct gfs2_sbd *sdp, int *ro_mount)
 {
 	FILE *fp;
@@ -161,6 +179,7 @@ int is_pathname_mounted(struct gfs2_sbd *sdp, int *ro_mount)
 		return 0;
 	if (hasmntopt(mnt, MNTOPT_RO))
                *ro_mount = 1;
+	sdp->secontext = copy_context_opt(mnt);
 	return 1; /* mounted */
 }
 
@@ -319,7 +338,7 @@ int mount_gfs2_meta(struct gfs2_sbd *sdp)
 	sigaction(SIGCONT, &sa, NULL);
 	sigaction(SIGUSR1, &sa, NULL);
 	sigaction(SIGUSR2, &sa, NULL);
-	ret = mount(sdp->path_name, sdp->metafs_path, "gfs2meta", 0, NULL);
+	ret = mount(sdp->path_name, sdp->metafs_path, "gfs2meta", 0, sdp->secontext);
 	if (ret) {
 		rmdir(sdp->metafs_path);
 		return -1;
-- 
1.9.3

[Cluster-devel] [PATCH RHEL6] libgfs2: Use a matching context mount option in mount_gfs2_meta

[Bob Peterson]

----- Original Message -----
> On a system with SELinux enabled, if a gfs2 file system is mounted with
> a context= option, the tools gfs2_quota, gfs2_tool, gfs2_grow and
> gfs2_jadd will fail with "Device or resource busy". This is due to
> SELinux failing the mount due to a mismatched context ("SELinux: mount
> invalid.  Same superblock, different security settings").
> 
> In order to work around this, parse the context option of the gfs2 mount
> point in is_pathname_mounted() and use it in mount_gfs2_meta().
> 
> Resolves: rhbz#1121693
> 
> Signed-off-by: Andrew Price <anprice@redhat.com>
> ---
>  gfs2/libgfs2/libgfs2.h |  1 +
>  gfs2/libgfs2/misc.c    | 21 ++++++++++++++++++++-
>  2 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
> index 9c20f11..25286d1 100644
> --- a/gfs2/libgfs2/libgfs2.h
> +++ b/gfs2/libgfs2/libgfs2.h
> @@ -217,6 +217,7 @@ struct gfs2_sbd {
>  
>  	int device_fd;
>  	int path_fd;
> +	char *secontext;
>  
>  	uint64_t sb_addr;
>  
> diff --git a/gfs2/libgfs2/misc.c b/gfs2/libgfs2/misc.c
> index 8e0ca6f..5ef4a2a 100644
> --- a/gfs2/libgfs2/misc.c
> +++ b/gfs2/libgfs2/misc.c
> @@ -100,6 +100,24 @@ int compute_constants(struct gfs2_sbd *sdp)
>  	return 0;
>  }
>  
> +/**
> + * Returns a duplicate of the 'context' mount option, or NULL if not found.
> + */
> +static char *copy_context_opt(struct mntent *mnt)
> +{
> +	char *ctx, *end;
> +
> +	ctx = hasmntopt(mnt, "context");
> +	if (ctx == NULL)
> +		return NULL;
> +
> +	end = strchr(ctx, ',');
> +	if (end == NULL)
> +		return NULL;
> +
> +	return strndup(ctx, end - ctx);
> +}
> +
>  int is_pathname_mounted(struct gfs2_sbd *sdp, int *ro_mount)
>  {
>  	FILE *fp;
> @@ -161,6 +179,7 @@ int is_pathname_mounted(struct gfs2_sbd *sdp, int
> *ro_mount)
>  		return 0;
>  	if (hasmntopt(mnt, MNTOPT_RO))
>                 *ro_mount = 1;
> +	sdp->secontext = copy_context_opt(mnt);
>  	return 1; /* mounted */
>  }
>  
> @@ -319,7 +338,7 @@ int mount_gfs2_meta(struct gfs2_sbd *sdp)
>  	sigaction(SIGCONT, &sa, NULL);
>  	sigaction(SIGUSR1, &sa, NULL);
>  	sigaction(SIGUSR2, &sa, NULL);
> -	ret = mount(sdp->path_name, sdp->metafs_path, "gfs2meta", 0, NULL);
> +	ret = mount(sdp->path_name, sdp->metafs_path, "gfs2meta", 0,
> sdp->secontext);
>  	if (ret) {
>  		rmdir(sdp->metafs_path);
>  		return -1;
> --
> 1.9.3
> 
> 
Hi,

Is this a memory leak (albeit a small one) or did I miss something?
I don't see where the memory allocate by strndup is ever freed.

Regards,

Bob Peterson
Red Hat File Systems

[Cluster-devel] [PATCH RHEL6] libgfs2: Use a matching context mount option in mount_gfs2_meta

[Andrew Price]

On 27/02/15 17:17, Bob Peterson wrote:
> ----- Original Message -----
>> On a system with SELinux enabled, if a gfs2 file system is mounted with
>> a context= option, the tools gfs2_quota, gfs2_tool, gfs2_grow and
>> gfs2_jadd will fail with "Device or resource busy". This is due to
>> SELinux failing the mount due to a mismatched context ("SELinux: mount
>> invalid.  Same superblock, different security settings").
>>
>> In order to work around this, parse the context option of the gfs2 mount
>> point in is_pathname_mounted() and use it in mount_gfs2_meta().

> Hi,
>
> Is this a memory leak (albeit a small one) or did I miss something?
> I don't see where the memory allocate by strndup is ever freed.

Well spotted. It is a tiny leak, but it couldn't hurt to plug it. I've 
checked that it isn't present in the upstream version of the patch so 
it's just this version which needs to be fixed. I'll send a follow-up 
patch shortly.

Thanks,
Andy

[Cluster-devel] [PATCH] libgfs2: Make sure secontext gets freed

[Andrew Price]

Free sbd.secontext after is_pathname_mounted allocates it. As the RHEL6
metafs code has no construct/destruct model similar to upstream, the
secontext string has to be freed in a few places additional to
cleanup_metafs.

Resolves: #1121693

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/fsck/initialize.c | 1 +
 gfs2/libgfs2/misc.c    | 1 +
 gfs2/tool/misc.c       | 2 ++
 gfs2/tool/tune.c       | 4 ++++
 4 files changed, 8 insertions(+)

diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
index 9abdf0c..0c374eb 100644
--- a/gfs2/fsck/initialize.c
+++ b/gfs2/fsck/initialize.c
@@ -1566,6 +1566,7 @@ int initialize(struct gfs2_sbd *sdp, int force_check, int preen,
 			sizeof(sdp->device_name));
 		sdp->path_name = sdp->device_name; /* This gets overwritten */
 		is_mounted = is_pathname_mounted(sdp, &ro);
+		free(sdp->secontext);
 		/* If the device is busy, but not because it's mounted, fail.
 		   This protects against cases where the file system is LVM
 		   and perhaps mounted on a different node. */
diff --git a/gfs2/libgfs2/misc.c b/gfs2/libgfs2/misc.c
index 5ef4a2a..955e141 100644
--- a/gfs2/libgfs2/misc.c
+++ b/gfs2/libgfs2/misc.c
@@ -301,6 +301,7 @@ void cleanup_metafs(struct gfs2_sbd *sdp)
 	sigaction(SIGUSR2, &sa, NULL);
 	metafs_interrupted = 0;
 	remove_mtab_entry(sdp);
+	free(sdp->secontext);
 }
 
 static void sighandler(int error)
diff --git a/gfs2/tool/misc.c b/gfs2/tool/misc.c
index 37c81cd..506f590 100644
--- a/gfs2/tool/misc.c
+++ b/gfs2/tool/misc.c
@@ -324,6 +324,7 @@ void do_withdraw(int argc, char **argv)
 	if (optind == argc)
 		die("Usage: gfs2_tool withdraw <mountpoint>\n");
 
+	sbd.secontext = NULL;
 	sbd.path_name = argv[optind];
 	if (check_for_gfs2(&sbd)) {
 		if (errno == EINVAL)
@@ -345,5 +346,6 @@ void do_withdraw(int argc, char **argv)
 				strerror(errno));
 		exit(-1);
 	}
+	free(sbd.secontext);
 }
 
diff --git a/gfs2/tool/tune.c b/gfs2/tool/tune.c
index 4666291..92a7b6f 100644
--- a/gfs2/tool/tune.c
+++ b/gfs2/tool/tune.c
@@ -46,6 +46,7 @@ get_tune(int argc, char **argv)
 	if (optind == argc)
 		die( _("Usage: gfs2_tool gettune <mountpoint>\n"));
 
+	sbd.secontext = NULL;
 	sbd.path_name = argv[optind];
 	if (check_for_gfs2(&sbd)) {
 		if (errno == EINVAL)
@@ -87,6 +88,7 @@ get_tune(int argc, char **argv)
 			printf("%s = %s\n", de->d_name, get_sysfs(fs, path));
 	}
 	closedir(d);
+	free(sbd.secontext);
 }
 
 /**
@@ -105,6 +107,7 @@ set_tune(int argc, char **argv)
 	char *fs;
 	struct gfs2_sbd sbd;
 
+	sbd.secontext = NULL;
 	if (optind == argc)
 		die( _("Usage: gfs2_tool settune <mountpoint> <parameter> <value>\n"));
 	sbd.path_name = argv[optind++];
@@ -141,4 +144,5 @@ set_tune(int argc, char **argv)
 				param, strerror(errno));
 		exit(-1);
 	}
+	free(sbd.secontext);
 }
-- 
1.9.3