linux-ext4 - Re: Problem with data=ordered ext4 mount option in linux-next

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d05a95a9-0bbd-3495-2b81-18673909edd4@gmail.com>
Date:   Fri, 17 Dec 2021 19:26:30 +0100
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Lukas Czerner <lczerner@...hat.com>
Cc:     linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: Problem with data=ordered ext4 mount option in linux-next

On 17.12.2021 18:02, Heiner Kallweit wrote:
> On 17.12.2021 16:34, Heiner Kallweit wrote:
>> On 17.12.2021 16:24, Lukas Czerner wrote:
>>> On Fri, Dec 17, 2021 at 04:11:32PM +0100, Heiner Kallweit wrote:
>>>> On linux-next systemd-remount-fs complains about an invalid mount option
>>>> here, resulting in a r/o root fs. After playing with the mount options
>>>> it turned out that data=ordered causes the problem. linux-next from Dec
>>>> 1st was ok, so it seems to be related to the new mount API patches.
>>>>
>>>> At a first glance I saw no obvious problem, the following looks good.
>>>> Maybe you have an idea where to look ..
>>>>
>>>> static const struct constant_table ext4_param_data[] = {
>>>> 	{"journal",	EXT4_MOUNT_JOURNAL_DATA},
>>>> 	{"ordered",	EXT4_MOUNT_ORDERED_DATA},
>>>> 	{"writeback",	EXT4_MOUNT_WRITEBACK_DATA},
>>>> 	{}
>>>> };
>>>>
>>>> 	fsparam_enum	("data",		Opt_data, ext4_param_data),
>>>>
>>>
>>> Thank you for the report!
>>>
>>> The ext4 mount has been reworked to use the new mount api and the work
>>> has been applied to linux-next couple of days ago so I definitelly
>>> assume there is a bug in there that I've missed. I will be looking into
>>> it.
>>>
>>> Can you be a little bit more specific about how to reproduce the problem
>>> as well as the error it generates in the logs ? Any other mount options
>>> used simultaneously, non-default file system features, or mount options
>>> stored within the superblock ?
>>>
>>> Can you reproduce it outside of the systemd unit, say a script ?
>>>
>> Yes:
>>
>> [root@...ac ~]# mount -o remount,data=ordered /
>> mount: /: mount point not mounted or bad option.
>> [root@...ac ~]# mount -o remount,discard /
>> [root@...ac ~]#
>>
>> "systemctl status systemd-remount-fs" shows the same error.
>>
>> Following options I had in my fstab (ext4 fs):
>> rw,relatime,data=ordered,discard
>>
>> No non-default system features.
>>
>>> Thanks!
>>> -Lukas
>>>
>> Heiner
> 
> Sorry, should have looked at dmesg earlier. There I see:
> EXT4-fs: Cannot change data mode on remount
> Message seems to be triggered from ext4_check_opt_consistency().
> Not sure why this error doesn't occur with old mount API.
> And actually I don't change the data mode.

Based on the old API code: Maybe we need something like this?
At least it works for me.

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index b72d989b7..9ec7e526c 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2821,7 +2821,9 @@ static int ext4_check_opt_consistency(struct fs_context *fc,
                                 "Remounting file system with no journal "
                                 "so ignoring journalled data option");
                        ctx_clear_mount_opt(ctx, EXT4_MOUNT_DATA_FLAGS);
-               } else if (ctx->mask_s_mount_opt & EXT4_MOUNT_DATA_FLAGS) {
+               } else if (ctx->mask_s_mount_opt & EXT4_MOUNT_DATA_FLAGS &&
+                          (ctx->vals_s_mount_opt & EXT4_MOUNT_DATA_FLAGS) !=
+                          (sbi->s_mount_opt & EXT4_MOUNT_DATA_FLAGS)) {
                        ext4_msg(NULL, KERN_ERR, "Cannot change data mode "
                                 "on remount");
                        return -EINVAL;