diff mbox series

mkfs.ubifs: fix xattr scanning for builds with selinux support

Message ID 20231215114824.15236-1-goliath@infraroot.at
State Accepted
Delegated to: David Oberhollenzer
Headers show
Series mkfs.ubifs: fix xattr scanning for builds with selinux support | expand

Commit Message

David Oberhollenzer Dec. 15, 2023, 11:48 a.m. UTC
mkfs.uibfs can add Selinux xattrs from a labeling file using
libselinux to parse it. The commit that added this feature simply
introduced a separate function, inode_add_selinux_xattr, which is
called instead of inode_add_xattr. If no --selinux argument is
specified for mkfs.ubifs, this is a no-op.

The problem is, that this breaks xattr scanning for any build with
Selinux enabled. The Selinux version is always called and it does
not scan for xattrs on the filesystem, or dispatch to the original.

This commit fixes the xattr scanning behavior. We unconditionally call
both functions (they each have no-op implementations if the feature
is missing) and in the regular xattr scanning code, we skip selinux
attributes, if the --selinux option was given.

Fixes: f1feccec5ad8 ("mkfs.ubifs: Implement selinux labelling support")

Signed-off-by: David Oberhollenzer <goliath@infraroot.at>
---
 ubifs-utils/mkfs.ubifs/mkfs.ubifs.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

Comments

Zhihao Cheng Dec. 16, 2023, 4:42 a.m. UTC | #1
在 2023/12/15 19:48, David Oberhollenzer 写道:
> mkfs.uibfs can add Selinux xattrs from a labeling file using
> libselinux to parse it. The commit that added this feature simply
> introduced a separate function, inode_add_selinux_xattr, which is
> called instead of inode_add_xattr. If no --selinux argument is
> specified for mkfs.ubifs, this is a no-op.
>
> The problem is, that this breaks xattr scanning for any build with
> Selinux enabled. The Selinux version is always called and it does
> not scan for xattrs on the filesystem, or dispatch to the original.
>
> This commit fixes the xattr scanning behavior. We unconditionally call
> both functions (they each have no-op implementations if the feature
> is missing) and in the regular xattr scanning code, we skip selinux
> attributes, if the --selinux option was given.
>
> Fixes: f1feccec5ad8 ("mkfs.ubifs: Implement selinux labelling support")
>
> Signed-off-by: David Oberhollenzer <goliath@infraroot.at>
> ---
>   ubifs-utils/mkfs.ubifs/mkfs.ubifs.c | 22 ++++++++++++++--------
>   1 file changed, 14 insertions(+), 8 deletions(-)

Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>

>
> diff --git a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
> index 15e6bdc..8f8d40b 100644
> --- a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
> +++ b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
> @@ -56,7 +56,6 @@
>   #ifdef WITH_SELINUX
>   #define XATTR_NAME_SELINUX "security.selinux"
>   static struct selabel_handle *sehnd;
> -static char *secontext;
>   #endif
>   
>   /**
> @@ -1389,6 +1388,15 @@ static int inode_add_xattr(struct ubifs_ino_node *host_ino,
>   			continue;
>   		}
>   
> +#ifdef WITH_SELINUX
> +		/*
> +		  Ignore selinux attributes if we have a label file, they are
> +		  instead provided by inode_add_selinux_xattr.
> +		 */
> +		if (!strcmp(name, XATTR_NAME_SELINUX) && context && sehnd)
> +			continue;
> +#endif
> +
>   		ret = add_xattr(host_ino, st, inum, name, attrbuf, attrsize);
>   		if (ret < 0)
>   			goto out_free;
> @@ -1413,12 +1421,10 @@ static int inode_add_selinux_xattr(struct ubifs_ino_node *host_ino,
>   	char *sepath = NULL;
>   	char *name;
>   	unsigned int con_size;
> +	char *secontext;
>   
> -	if (!context || !sehnd) {
> -		secontext = NULL;
> -		con_size = 0;
> +	if (!context || !sehnd)
>   		return 0;
> -	}
>   
>   	if (path_name[strlen(root)] == '/')
>   		sepath = strdup(&path_name[strlen(root)]);
> @@ -1595,11 +1601,11 @@ static int add_inode(struct stat *st, ino_t inum, void *data,
>   	len = UBIFS_INO_NODE_SZ + data_len;
>   
>   	if (xattr_path) {
> -#ifdef WITH_SELINUX
>   		ret = inode_add_selinux_xattr(ino, xattr_path, st, inum);
> -#else
> +		if (ret < 0)
> +			return ret;
> +
>   		ret = inode_add_xattr(ino, xattr_path, st, inum);
> -#endif
>   		if (ret < 0)
>   			return ret;
>   	}
David Oberhollenzer Dec. 24, 2023, 8:18 p.m. UTC | #2
applied to mtd-utils.git master.

Thanks,

David
diff mbox series

Patch

diff --git a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
index 15e6bdc..8f8d40b 100644
--- a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
+++ b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
@@ -56,7 +56,6 @@ 
 #ifdef WITH_SELINUX
 #define XATTR_NAME_SELINUX "security.selinux"
 static struct selabel_handle *sehnd;
-static char *secontext;
 #endif
 
 /**
@@ -1389,6 +1388,15 @@  static int inode_add_xattr(struct ubifs_ino_node *host_ino,
 			continue;
 		}
 
+#ifdef WITH_SELINUX
+		/*
+		  Ignore selinux attributes if we have a label file, they are
+		  instead provided by inode_add_selinux_xattr.
+		 */
+		if (!strcmp(name, XATTR_NAME_SELINUX) && context && sehnd)
+			continue;
+#endif
+
 		ret = add_xattr(host_ino, st, inum, name, attrbuf, attrsize);
 		if (ret < 0)
 			goto out_free;
@@ -1413,12 +1421,10 @@  static int inode_add_selinux_xattr(struct ubifs_ino_node *host_ino,
 	char *sepath = NULL;
 	char *name;
 	unsigned int con_size;
+	char *secontext;
 
-	if (!context || !sehnd) {
-		secontext = NULL;
-		con_size = 0;
+	if (!context || !sehnd)
 		return 0;
-	}
 
 	if (path_name[strlen(root)] == '/')
 		sepath = strdup(&path_name[strlen(root)]);
@@ -1595,11 +1601,11 @@  static int add_inode(struct stat *st, ino_t inum, void *data,
 	len = UBIFS_INO_NODE_SZ + data_len;
 
 	if (xattr_path) {
-#ifdef WITH_SELINUX
 		ret = inode_add_selinux_xattr(ino, xattr_path, st, inum);
-#else
+		if (ret < 0)
+			return ret;
+
 		ret = inode_add_xattr(ino, xattr_path, st, inum);
-#endif
 		if (ret < 0)
 			return ret;
 	}