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

[Petr Lautrbach <lautrbach@redhat.com>]

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
>>
>>