[PATCH v2] restorecon: Only log error on readonly fs (bsc#1232226)

[PATCH v2] restorecon: Only log error on readonly fs (bsc#1232226)

[Cathy Hu]

Problem:

Before this change, when restorecon encountered a read-only filesystem,
it would set the error code to 255, not continue traversing, but
continue with other paths:
"restorecon: Could not set context for <path>:  Read-only file system"

For callers there is no way to distinguish from that error code if it
encountered a serious error, or a read-only fs was not relabelled, or
both. However, not failing when a read-only fs is traversed is
acceptable in most if not all cases.

This caused issues when e.g. restorecon was used in autorelabel
mechanisms during boot to relabel the file system. When it encountered
a read-only BTRFS subvolume, the return code would be set to 255 and
we had to fail the relabel service, as it might be a more severe issue.

Behaviour change in this commit:

With this change, restorecon logs the encounter of a ro file system, continues and finishes with return code 0:
"Read only filesystem, relabel not possible: <path>"

Other mechanims that were considered to fix this issue and why they were
not used:

- -x skip all subvolumes: break cases of subvolumes which aren't mounted explicitly but still need to be relabelled
- caller generates skippable locations and these get automatically fed
  into restorecon: error prone and needs to be manually maintained in
certain scenarios
- caller ignores non-zero return code: not good idea, it would cover up
  issues that would be hard to debug in the future
- skip only ro btrfs subvolumes: I started writing a poc and discovered it is not easily
  possible without either pulling libbtrfs in as dependency or having
  a lot of ioctl calls which likely will lead to performance degradation
  or tracking the btrfs status inside of restorecon, which would be a
  big code change for a problem that might be also there on other file
  systems

Discussion and context:

- "Question regarding restorecon and btrfs read-only snapshots" Thread:
https://lore.kernel.org/selinux/98f87fd6-6d3e-4539-ad8f-1a0dc09aa890@suse.de/

- (open)SUSE bug: https://bugzilla.suse.com/show_bug.cgi?id=1232226

Signed-off-by: Cathy Hu <cahu@suse.de>
---
 libselinux/src/selinux_restorecon.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
index 8fadf4d2..e8545e27 100644
--- a/libselinux/src/selinux_restorecon.c
+++ b/libselinux/src/selinux_restorecon.c
@@ -774,10 +774,14 @@ static int restorecon_sb(const char *pathname, const struct stat *sb,
 		if (!flags->nochange) {
 			if (lsetfilecon(pathname, newcon) < 0) {
 				/* Ignore files removed during relabeling if ignore_noent is set */
-				if (flags->ignore_noent && errno == ENOENT)
+				if (flags->ignore_noent && errno == ENOENT) {
 					goto out;
-				else
+				} else if (errno == EROFS) {
+					selinux_log(SELINUX_INFO, "Read only filesystem, relabel not possible: %s\n", pathname);
+					goto out;
+				} else {
 					goto err;
+				}
 			}
 
 			updated = true;
-- 
2.53.0

Re: [PATCH v2] restorecon: Only log error on readonly fs (bsc#1232226)

[James Carter]

On Thu, Apr 30, 2026 at 7:34 AM Cathy Hu <cahu@suse.de> wrote:
>
> Problem:
>
> Before this change, when restorecon encountered a read-only filesystem,
> it would set the error code to 255, not continue traversing, but
> continue with other paths:
> "restorecon: Could not set context for <path>:  Read-only file system"
>
> For callers there is no way to distinguish from that error code if it
> encountered a serious error, or a read-only fs was not relabelled, or
> both. However, not failing when a read-only fs is traversed is
> acceptable in most if not all cases.
>
> This caused issues when e.g. restorecon was used in autorelabel
> mechanisms during boot to relabel the file system. When it encountered
> a read-only BTRFS subvolume, the return code would be set to 255 and
> we had to fail the relabel service, as it might be a more severe issue.
>
> Behaviour change in this commit:
>
> With this change, restorecon logs the encounter of a ro file system, continues and finishes with return code 0:
> "Read only filesystem, relabel not possible: <path>"
>
> Other mechanims that were considered to fix this issue and why they were
> not used:
>
> - -x skip all subvolumes: break cases of subvolumes which aren't mounted explicitly but still need to be relabelled
> - caller generates skippable locations and these get automatically fed
>   into restorecon: error prone and needs to be manually maintained in
> certain scenarios
> - caller ignores non-zero return code: not good idea, it would cover up
>   issues that would be hard to debug in the future
> - skip only ro btrfs subvolumes: I started writing a poc and discovered it is not easily
>   possible without either pulling libbtrfs in as dependency or having
>   a lot of ioctl calls which likely will lead to performance degradation
>   or tracking the btrfs status inside of restorecon, which would be a
>   big code change for a problem that might be also there on other file
>   systems
>
> Discussion and context:
>
> - "Question regarding restorecon and btrfs read-only snapshots" Thread:
> https://lore.kernel.org/selinux/98f87fd6-6d3e-4539-ad8f-1a0dc09aa890@suse.de/
>
> - (open)SUSE bug: https://bugzilla.suse.com/show_bug.cgi?id=1232226
>
> Signed-off-by: Cathy Hu <cahu@suse.de>

Acked-by: James Carter <jwcart2@gmail.com>

> ---
>  libselinux/src/selinux_restorecon.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
> index 8fadf4d2..e8545e27 100644
> --- a/libselinux/src/selinux_restorecon.c
> +++ b/libselinux/src/selinux_restorecon.c
> @@ -774,10 +774,14 @@ static int restorecon_sb(const char *pathname, const struct stat *sb,
>                 if (!flags->nochange) {
>                         if (lsetfilecon(pathname, newcon) < 0) {
>                                 /* Ignore files removed during relabeling if ignore_noent is set */
> -                               if (flags->ignore_noent && errno == ENOENT)
> +                               if (flags->ignore_noent && errno == ENOENT) {
>                                         goto out;
> -                               else
> +                               } else if (errno == EROFS) {
> +                                       selinux_log(SELINUX_INFO, "Read only filesystem, relabel not possible: %s\n", pathname);
> +                                       goto out;
> +                               } else {
>                                         goto err;
> +                               }
>                         }
>
>                         updated = true;
> --
> 2.53.0
>
>

Re: [PATCH v2] restorecon: Only log error on readonly fs (bsc#1232226)

[Petr Lautrbach]

James Carter <jwcart2@gmail.com> writes:

> On Thu, Apr 30, 2026 at 7:34 AM Cathy Hu <cahu@suse.de> wrote:
>>
>> Problem:
>>
>> Before this change, when restorecon encountered a read-only filesystem,
>> it would set the error code to 255, not continue traversing, but
>> continue with other paths:
>> "restorecon: Could not set context for <path>:  Read-only file system"
>>
>> For callers there is no way to distinguish from that error code if it
>> encountered a serious error, or a read-only fs was not relabelled, or
>> both. However, not failing when a read-only fs is traversed is
>> acceptable in most if not all cases.
>>
>> This caused issues when e.g. restorecon was used in autorelabel
>> mechanisms during boot to relabel the file system. When it encountered
>> a read-only BTRFS subvolume, the return code would be set to 255 and
>> we had to fail the relabel service, as it might be a more severe issue.
>>
>> Behaviour change in this commit:
>>
>> With this change, restorecon logs the encounter of a ro file system, continues and finishes with return code 0:
>> "Read only filesystem, relabel not possible: <path>"
>>
>> Other mechanims that were considered to fix this issue and why they were
>> not used:
>>
>> - -x skip all subvolumes: break cases of subvolumes which aren't mounted explicitly but still need to be relabelled
>> - caller generates skippable locations and these get automatically fed
>>   into restorecon: error prone and needs to be manually maintained in
>> certain scenarios
>> - caller ignores non-zero return code: not good idea, it would cover up
>>   issues that would be hard to debug in the future
>> - skip only ro btrfs subvolumes: I started writing a poc and discovered it is not easily
>>   possible without either pulling libbtrfs in as dependency or having
>>   a lot of ioctl calls which likely will lead to performance degradation
>>   or tracking the btrfs status inside of restorecon, which would be a
>>   big code change for a problem that might be also there on other file
>>   systems
>>
>> Discussion and context:
>>
>> - "Question regarding restorecon and btrfs read-only snapshots" Thread:
>> https://lore.kernel.org/selinux/98f87fd6-6d3e-4539-ad8f-1a0dc09aa890@suse.de/
>>
>> - (open)SUSE bug: https://bugzilla.suse.com/show_bug.cgi?id=1232226
>>
>> Signed-off-by: Cathy Hu <cahu@suse.de>
>
> Acked-by: James Carter <jwcart2@gmail.com>


Merged, thanks!

>> ---
>>  libselinux/src/selinux_restorecon.c | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
>> index 8fadf4d2..e8545e27 100644
>> --- a/libselinux/src/selinux_restorecon.c
>> +++ b/libselinux/src/selinux_restorecon.c
>> @@ -774,10 +774,14 @@ static int restorecon_sb(const char *pathname, const struct stat *sb,
>>                 if (!flags->nochange) {
>>                         if (lsetfilecon(pathname, newcon) < 0) {
>>                                 /* Ignore files removed during relabeling if ignore_noent is set */
>> -                               if (flags->ignore_noent && errno == ENOENT)
>> +                               if (flags->ignore_noent && errno == ENOENT) {
>>                                         goto out;
>> -                               else
>> +                               } else if (errno == EROFS) {
>> +                                       selinux_log(SELINUX_INFO, "Read only filesystem, relabel not possible: %s\n", pathname);
>> +                                       goto out;
>> +                               } else {
>>                                         goto err;
>> +                               }
>>                         }
>>
>>                         updated = true;
>> --
>> 2.53.0
>>
>>