[PATCH v2 1/2] libselinux: prioritize local literal fcontext definitions

[PATCH v2 1/2] libselinux: prioritize local literal fcontext definitions

[Christian Göttsche]

From: Christian Göttsche <cgzones@googlemail.com>

For literal file context definitions respect overrides from homedirs or
local configurations by ordering them first.

Also retain the order regarding the file kind for backward
compatibility.

Fixes: 92306daf ("libselinux: rework selabel_file(5) database")
Reported-by: Paul Holzinger
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2360183
Suggested-by: James Carter <jwcart2@gmail.com>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v2: drop ordering based in file type, thanks Jim
---
 libselinux/src/label_file.c       | 5 +++--
 libselinux/src/label_file.h       | 7 +++++--
 libselinux/src/selinux_internal.h | 2 ++
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
index 5d73fb84..61a9ddb6 100644
--- a/libselinux/src/label_file.c
+++ b/libselinux/src/label_file.c
@@ -480,7 +480,7 @@ static int load_mmap_ctxarray(struct mmap_area *mmap_area, const char *path, str
 	return 0;
 }
 
-static int load_mmap_literal_spec(struct mmap_area *mmap_area, bool validating,
+static int load_mmap_literal_spec(struct mmap_area *mmap_area, bool validating, uint8_t inputno,
 				  struct literal_spec *lspec, const struct context_array *ctx_array)
 {
 	uint32_t data_u32, ctx_id;
@@ -489,6 +489,7 @@ static int load_mmap_literal_spec(struct mmap_area *mmap_area, bool validating,
 	int rc;
 
 	lspec->from_mmap = true;
+	lspec->inputno = inputno;
 
 
 	/*
@@ -742,7 +743,7 @@ static int load_mmap_spec_node(struct mmap_area *mmap_area, const char *path, bo
 		node->literal_specs_alloc = lspec_num;
 
 		for (uint32_t i = 0; i < lspec_num; i++) {
-			rc = load_mmap_literal_spec(mmap_area, validating, &node->literal_specs[i], ctx_array);
+			rc = load_mmap_literal_spec(mmap_area, validating, inputno, &node->literal_specs[i], ctx_array);
 			if (rc)
 				return -1;
 		}
diff --git a/libselinux/src/label_file.h b/libselinux/src/label_file.h
index 67db78e5..d1a0713a 100644
--- a/libselinux/src/label_file.h
+++ b/libselinux/src/label_file.h
@@ -96,6 +96,7 @@ struct literal_spec {
 	char *regex_str;			/* original regular expression string for diagnostics */
 	char *literal_match;			/* simplified string from regular expression */
 	uint16_t prefix_len;			/* length of fixed path prefix, i.e. length of the literal match */
+	uint8_t inputno;			/* Input number of source file */
 	uint8_t file_kind;			/* file type */
 	bool any_matches;			/* whether any pathname match */
 	bool from_mmap;				/* whether this spec is from an mmap of the data */
@@ -367,8 +368,9 @@ static inline int compare_literal_spec(const void *p1, const void *p2)
 	if (ret)
 		return ret;
 
-	/* Order wildcard mode (0) last */
-	return (l1->file_kind < l2->file_kind) - (l1->file_kind > l2->file_kind);
+	/* Order by input number (higher number means added later, means higher priority) */
+	ret = spaceship_cmp(l1->inputno, l2->inputno);
+	return -ret;
 }
 
 static inline int compare_spec_node(const void *p1, const void *p2)
@@ -754,6 +756,7 @@ static int insert_spec(const struct selabel_handle *rec, struct saved_data *data
 			.regex_str = regex,
 			.prefix_len = prefix_len,
 			.literal_match = literal_regex,
+			.inputno = inputno,
 			.file_kind = file_kind,
 			.any_matches = false,
 			.lr.ctx_raw = context,
diff --git a/libselinux/src/selinux_internal.h b/libselinux/src/selinux_internal.h
index 964b8418..3fe7d4c3 100644
--- a/libselinux/src/selinux_internal.h
+++ b/libselinux/src/selinux_internal.h
@@ -150,4 +150,6 @@ static inline void fclose_errno_safe(FILE *stream)
 # define unlikely(x)			(x)
 #endif /* __GNUC__ */
 
+#define spaceship_cmp(a, b)		(((a) > (b)) - ((a) < (b)))
+
 #endif /* SELINUX_INTERNAL_H_ */
-- 
2.49.0

[PATCH v2 2/2] libselinux: retain LIFO order for path substitutions

[Christian Göttsche]

From: Christian Göttsche <cgzones@googlemail.com>

Store the path substitutions in LIFO order as in previous versions.

Fixes: 4d436e4b ("libselinux: use vector instead of linked list for substitutions")
Reported-by: Dominick Grift <dominick.grift@defensec.nl>
Link: https://lore.kernel.org/selinux/87ldqftsxd.fsf@defensec.nl/
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v2: add patch
---
 libselinux/src/label_file.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
index 61a9ddb6..b785eab6 100644
--- a/libselinux/src/label_file.c
+++ b/libselinux/src/label_file.c
@@ -1350,6 +1350,15 @@ static int selabel_subs_init(const char *path, struct selabel_digest *digest,
 	if (digest_add_specfile(digest, cfg, NULL, sb.st_size, path) < 0)
 		goto err;
 
+	/* LIFO order for backward compatibility */
+	for (uint32_t i = 0; i < tmp_num/2; i++) {
+		struct selabel_sub swap;
+
+		swap = tmp[i];
+		tmp[i] = tmp[tmp_num - i - 1];
+		tmp[tmp_num - i - 1] = swap;
+	}
+
 	*out_subs = tmp;
 	*out_num = tmp_num;
 	*out_alloc = tmp_alloc;
-- 
2.49.0

Re: [PATCH v2 2/2] libselinux: retain LIFO order for path substitutions

[Petr Lautrbach]

Christian Göttsche <cgoettsche@seltendoof.de> writes:

> From: Christian Göttsche <cgzones@googlemail.com>
>
> Store the path substitutions in LIFO order as in previous versions.
>
> Fixes: 4d436e4b ("libselinux: use vector instead of linked list for substitutions")
> Reported-by: Dominick Grift <dominick.grift@defensec.nl>
> Link: https://lore.kernel.org/selinux/87ldqftsxd.fsf@defensec.nl/
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
> v2: add patch
> ---
>  libselinux/src/label_file.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
> index 61a9ddb6..b785eab6 100644
> --- a/libselinux/src/label_file.c
> +++ b/libselinux/src/label_file.c
> @@ -1350,6 +1350,15 @@ static int selabel_subs_init(const char *path, struct selabel_digest *digest,
>  	if (digest_add_specfile(digest, cfg, NULL, sb.st_size, path) < 0)
>  		goto err;
>  
> +	/* LIFO order for backward compatibility */
> +	for (uint32_t i = 0; i < tmp_num/2; i++) {
> +		struct selabel_sub swap;
> +
> +		swap = tmp[i];
> +		tmp[i] = tmp[tmp_num - i - 1];
> +		tmp[tmp_num - i - 1] = swap;
> +	}
> +
>  	*out_subs = tmp;
>  	*out_num = tmp_num;
>  	*out_alloc = tmp_alloc;
> -- 
> 2.49.0

It fixes the original reproducer:

root@fedora:/# cat /etc/selinux/fedora-selinux/contexts/files/file_contexts.subs
/new_root /
/new_root/foo /usr/bin
root@fedora:/# matchpathcon /new_root /new_root/foo
/new_root       system_u:object_r:root_t:s0
/new_root/foo   system_u:object_r:bin_t:s0

but it's important to say it depends on the order of entries in .subst:

root@fedora:/# cat /etc/selinux/fedora-selinux/contexts/files/file_contexts.subs
/new_root/foo /usr/bin
/new_root /
root@fedora:/# matchpathcon /new_root /new_root/foo
/new_root       system_u:object_r:root_t:s0
/new_root/foo   system_u:object_r:etc_runtime_t:s0


it looks like it was like that even before with 3.8:

[root@fedora-41 /]# cat /etc/selinux/targeted/contexts/files/file_contexts.subs
/new_root /
/new_root/foo /usr/bin
[root@fedora-41 /]# matchpathcon /new_root /new_root/foo
/new_root       system_u:object_r:root_t:s0
/new_root/foo   system_u:object_r:bin_t:s0

[root@fedora-41 /]# cat /etc/selinux/targeted/contexts/files/file_contexts.subs
/new_root/foo /usr/bin
/new_root /
[root@fedora-41 /]# matchpathcon /new_root /new_root/foo
/new_root       system_u:object_r:root_t:s0
/new_root/foo   system_u:object_r:etc_runtime_t:s0

It would be great to have this behavior documented, but it's a different
issue.

If there's no other objection I'll merge tomorrow before 3.9-rc1.

Acked-by: Petr Lautrbach <lautrbach@redhat.com>

Re: [PATCH v2 2/2] libselinux: retain LIFO order for path substitutions

[Dominick Grift]

Petr Lautrbach <lautrbach@redhat.com> writes:

> Christian Göttsche <cgoettsche@seltendoof.de> writes:
>
>> From: Christian Göttsche <cgzones@googlemail.com>
>>
>> Store the path substitutions in LIFO order as in previous versions.
>>
>> Fixes: 4d436e4b ("libselinux: use vector instead of linked list for substitutions")
>> Reported-by: Dominick Grift <dominick.grift@defensec.nl>
>> Link: https://lore.kernel.org/selinux/87ldqftsxd.fsf@defensec.nl/
>> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
>> ---
>> v2: add patch
>> ---
>>  libselinux/src/label_file.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
>> index 61a9ddb6..b785eab6 100644
>> --- a/libselinux/src/label_file.c
>> +++ b/libselinux/src/label_file.c
>> @@ -1350,6 +1350,15 @@ static int selabel_subs_init(const char *path, struct selabel_digest *digest,
>>  	if (digest_add_specfile(digest, cfg, NULL, sb.st_size, path) < 0)
>>  		goto err;
>>  
>> +	/* LIFO order for backward compatibility */
>> +	for (uint32_t i = 0; i < tmp_num/2; i++) {
>> +		struct selabel_sub swap;
>> +
>> +		swap = tmp[i];
>> +		tmp[i] = tmp[tmp_num - i - 1];
>> +		tmp[tmp_num - i - 1] = swap;
>> +	}
>> +
>>  	*out_subs = tmp;
>>  	*out_num = tmp_num;
>>  	*out_alloc = tmp_alloc;
>> -- 
>> 2.49.0
>
> It fixes the original reproducer:
>
> root@fedora:/# cat /etc/selinux/fedora-selinux/contexts/files/file_contexts.subs
> /new_root /
> /new_root/foo /usr/bin
> root@fedora:/# matchpathcon /new_root /new_root/foo
> /new_root       system_u:object_r:root_t:s0
> /new_root/foo   system_u:object_r:bin_t:s0
>
> but it's important to say it depends on the order of entries in .subst:
>
> root@fedora:/# cat /etc/selinux/fedora-selinux/contexts/files/file_contexts.subs
> /new_root/foo /usr/bin
> /new_root /
> root@fedora:/# matchpathcon /new_root /new_root/foo
> /new_root       system_u:object_r:root_t:s0
> /new_root/foo   system_u:object_r:etc_runtime_t:s0
>
>
> it looks like it was like that even before with 3.8:
>
> [root@fedora-41 /]# cat /etc/selinux/targeted/contexts/files/file_contexts.subs
> /new_root /
> /new_root/foo /usr/bin
> [root@fedora-41 /]# matchpathcon /new_root /new_root/foo
> /new_root       system_u:object_r:root_t:s0
> /new_root/foo   system_u:object_r:bin_t:s0
>
> [root@fedora-41 /]# cat /etc/selinux/targeted/contexts/files/file_contexts.subs
> /new_root/foo /usr/bin
> /new_root /
> [root@fedora-41 /]# matchpathcon /new_root /new_root/foo
> /new_root       system_u:object_r:root_t:s0
> /new_root/foo   system_u:object_r:etc_runtime_t:s0
>
> It would be great to have this behavior documented, but it's a different
> issue.
>
> If there's no other objection I'll merge tomorrow before 3.9-rc1.
>
> Acked-by: Petr Lautrbach <lautrbach@redhat.com>
>

I applied this patch and it addresses my issue (restores compatiblity).
I can also confirm that ordering the specs indeed also addresses the
issue at least in a simple test I did.

I do prefer that ordering shouldnt matter.

Thanks!

-- 
gpg --locate-keys dominick.grift@defensec.nl (wkd)
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
Dominick Grift
Mastodon: @kcinimod@defensec.nl

Re: [PATCH v2 2/2] libselinux: retain LIFO order for path substitutions

[James Carter]

On Tue, Jun 10, 2025 at 7:28 AM Dominick Grift
<dominick.grift@defensec.nl> wrote:
>
> Petr Lautrbach <lautrbach@redhat.com> writes:
>
> > Christian Göttsche <cgoettsche@seltendoof.de> writes:
> >
> >> From: Christian Göttsche <cgzones@googlemail.com>
> >>
> >> Store the path substitutions in LIFO order as in previous versions.
> >>
> >> Fixes: 4d436e4b ("libselinux: use vector instead of linked list for substitutions")
> >> Reported-by: Dominick Grift <dominick.grift@defensec.nl>
> >> Link: https://lore.kernel.org/selinux/87ldqftsxd.fsf@defensec.nl/
> >> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> >> ---
> >> v2: add patch
> >> ---
> >>  libselinux/src/label_file.c | 9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >>
> >> diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
> >> index 61a9ddb6..b785eab6 100644
> >> --- a/libselinux/src/label_file.c
> >> +++ b/libselinux/src/label_file.c
> >> @@ -1350,6 +1350,15 @@ static int selabel_subs_init(const char *path, struct selabel_digest *digest,
> >>      if (digest_add_specfile(digest, cfg, NULL, sb.st_size, path) < 0)
> >>              goto err;
> >>
> >> +    /* LIFO order for backward compatibility */
> >> +    for (uint32_t i = 0; i < tmp_num/2; i++) {
> >> +            struct selabel_sub swap;
> >> +
> >> +            swap = tmp[i];
> >> +            tmp[i] = tmp[tmp_num - i - 1];
> >> +            tmp[tmp_num - i - 1] = swap;
> >> +    }
> >> +
> >>      *out_subs = tmp;
> >>      *out_num = tmp_num;
> >>      *out_alloc = tmp_alloc;
> >> -- rc = cil_gen_node(db, ast_node, (struct cil_symtab_datum*)role, (hashtab_key_t)key, CIL_SYM_ROLES, CIL_ROLE);
if (rc != SEPOL_OK) {
if (rc == SEPOL_EEXIST) {
cil_destroy_role(role);
role = NULL;
} else {
goto exit;
}
}
> >> 2.49.0
> >
> > It fixes the original reproducer:
> >
> > root@fedora:/# cat /etc/selinux/fedora-selinux/contexts/files/file_contexts.subs
> > /new_root /
> > /new_root/foo /usr/bin
> > root@fedora:/# matchpathcon /new_root /new_root/foo
> > /new_root       system_u:object_r:root_t:s0
> > /new_root/foo   system_u:object_r:bin_t:s0
> >
> > but it's important to say it depends on the order of entries in .subst:
> >
> > root@fedora:/# cat /etc/selinux/fedora-selinux/contexts/files/file_contexts.subs
> > /new_root/foo /usr/bin
> > /new_root /
> > root@fedora:/# matchpathcon /new_root /new_root/foo
> > /new_root       system_u:object_r:root_t:s0
> > /new_root/foo   system_u:object_r:etc_runtime_t:s0
> >
> >
> > it looks like it was like that even before with 3.8:
> >
> > [root@fedora-41 /]# cat /etc/selinux/targeted/contexts/files/file_contexts.subs
> > /new_root /
> > /new_root/foo /usr/bin
> > [root@fedora-41 /]# matchpathcon /new_root /new_root/foo
> > /new_root       system_u:object_r:root_t:s0
> > /new_root/foo   system_u:object_r:bin_t:s0
> >
> > [root@fedora-41 /]# cat /etc/selinux/targeted/contexts/files/file_contexts.subs
> > /new_root/foo /usr/bin
> > /new_root /
> > [root@fedora-41 /]# matchpathcon /new_root /new_root/foo
> > /new_root       system_u:object_r:root_t:s0
> > /new_root/foo   system_u:object_r:etc_runtime_t:s0
> >
> > It would be great to have this behavior documented, but it's a different
> > issue.
> >
> > If there's no other objection I'll merge tomorrow before 3.9-rc1.
> >
> > Acked-by: Petr Lautrbach <lautrbach@redhat.com>
> >
>
> I applied this patch and it addresses my issue (restores compatiblity).
> I can also confirm that ordering the specs indeed also addresses the
> issue at least in a simple test I did.
>
> I do prefer that ordering shouldnt matter.
>
> Thanks!
>

If it fixes Dominick's issues, then I have no objection. I had planned
on verifying that it had fixed his problem, but now I don't have to.
Thanks everyone,
Jim

> --
> gpg --locate-keys dominick.grift@defensec.nl (wkd)
> Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
> Dominick Grift
> Mastodon: @kcinimod@defensec.nl
>

Re: [PATCH v2 1/2] libselinux: prioritize local literal fcontext definitions

[Petr Lautrbach]

Christian Göttsche <cgoettsche@seltendoof.de> writes:

> From: Christian Göttsche <cgzones@googlemail.com>
>
> For literal file context definitions respect overrides from homedirs or
> local configurations by ordering them first.
>
> Also retain the order regarding the file kind for backward
> compatibility.
>
> Fixes: 92306daf ("libselinux: rework selabel_file(5) database")
> Reported-by: Paul Holzinger
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2360183
> Suggested-by: James Carter <jwcart2@gmail.com>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Acked-by: Petr Lautrbach <lautrbach@redhat.com>



> ---
> v2: drop ordering based in file type, thanks Jim
> ---
>  libselinux/src/label_file.c       | 5 +++--
>  libselinux/src/label_file.h       | 7 +++++--
>  libselinux/src/selinux_internal.h | 2 ++
>  3 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
> index 5d73fb84..61a9ddb6 100644
> --- a/libselinux/src/label_file.c
> +++ b/libselinux/src/label_file.c
> @@ -480,7 +480,7 @@ static int load_mmap_ctxarray(struct mmap_area *mmap_area, const char *path, str
>  	return 0;
>  }
>  
> -static int load_mmap_literal_spec(struct mmap_area *mmap_area, bool validating,
> +static int load_mmap_literal_spec(struct mmap_area *mmap_area, bool validating, uint8_t inputno,
>  				  struct literal_spec *lspec, const struct context_array *ctx_array)
>  {
>  	uint32_t data_u32, ctx_id;
> @@ -489,6 +489,7 @@ static int load_mmap_literal_spec(struct mmap_area *mmap_area, bool validating,
>  	int rc;
>  
>  	lspec->from_mmap = true;
> +	lspec->inputno = inputno;
>  
>  
>  	/*
> @@ -742,7 +743,7 @@ static int load_mmap_spec_node(struct mmap_area *mmap_area, const char *path, bo
>  		node->literal_specs_alloc = lspec_num;
>  
>  		for (uint32_t i = 0; i < lspec_num; i++) {
> -			rc = load_mmap_literal_spec(mmap_area, validating, &node->literal_specs[i], ctx_array);
> +			rc = load_mmap_literal_spec(mmap_area, validating, inputno, &node->literal_specs[i], ctx_array);
>  			if (rc)
>  				return -1;
>  		}
> diff --git a/libselinux/src/label_file.h b/libselinux/src/label_file.h
> index 67db78e5..d1a0713a 100644
> --- a/libselinux/src/label_file.h
> +++ b/libselinux/src/label_file.h
> @@ -96,6 +96,7 @@ struct literal_spec {
>  	char *regex_str;			/* original regular expression string for diagnostics */
>  	char *literal_match;			/* simplified string from regular expression */
>  	uint16_t prefix_len;			/* length of fixed path prefix, i.e. length of the literal match */
> +	uint8_t inputno;			/* Input number of source file */
>  	uint8_t file_kind;			/* file type */
>  	bool any_matches;			/* whether any pathname match */
>  	bool from_mmap;				/* whether this spec is from an mmap of the data */
> @@ -367,8 +368,9 @@ static inline int compare_literal_spec(const void *p1, const void *p2)
>  	if (ret)
>  		return ret;
>  
> -	/* Order wildcard mode (0) last */
> -	return (l1->file_kind < l2->file_kind) - (l1->file_kind > l2->file_kind);
> +	/* Order by input number (higher number means added later, means higher priority) */
> +	ret = spaceship_cmp(l1->inputno, l2->inputno);
> +	return -ret;
>  }
>  
>  static inline int compare_spec_node(const void *p1, const void *p2)
> @@ -754,6 +756,7 @@ static int insert_spec(const struct selabel_handle *rec, struct saved_data *data
>  			.regex_str = regex,
>  			.prefix_len = prefix_len,
>  			.literal_match = literal_regex,
> +			.inputno = inputno,
>  			.file_kind = file_kind,
>  			.any_matches = false,
>  			.lr.ctx_raw = context,
> diff --git a/libselinux/src/selinux_internal.h b/libselinux/src/selinux_internal.h
> index 964b8418..3fe7d4c3 100644
> --- a/libselinux/src/selinux_internal.h
> +++ b/libselinux/src/selinux_internal.h
> @@ -150,4 +150,6 @@ static inline void fclose_errno_safe(FILE *stream)
>  # define unlikely(x)			(x)
>  #endif /* __GNUC__ */
>  
> +#define spaceship_cmp(a, b)		(((a) > (b)) - ((a) < (b)))
> +
>  #endif /* SELINUX_INTERNAL_H_ */
> -- 
> 2.49.0