* [PATCH] btrfs: reject log replay if there is unsupported RO flag
@ 2022-06-07 2:31 Qu Wenruo
2022-06-07 9:49 ` Filipe Manana
0 siblings, 1 reply; 3+ messages in thread
From: Qu Wenruo @ 2022-06-07 2:31 UTC (permalink / raw)
To: linux-btrfs; +Cc: stable
[BUG]
If we have a btrfs image with dirty log, along with an unsupported RO
compatible flag:
log_root 30474240
...
compat_flags 0x0
compat_ro_flags 0x40000003
( FREE_SPACE_TREE |
FREE_SPACE_TREE_VALID |
unknown flag: 0x40000000 )
Then even if we can only mount it RO, we will still cause metadata
update for log replay:
BTRFS info (device dm-1): flagging fs with big metadata feature
BTRFS info (device dm-1): using free space tree
BTRFS info (device dm-1): has skinny extents
BTRFS info (device dm-1): start tree-log replay
This is definitely against RO compact flag requirement.
[CAUSE]
RO compact flag only forces us to do RO mount, but we will still do log
replay for plain RO mount.
Thus this will result us to do log replay and update metadata.
This can be very problematic for new RO compat flag, for example older
kernel can not understand v2 cache, and if we allow metadata update on
RO mount and invalidate/corrupt v2 cache.
[FIX]
Just set the nologreplay flag if there is any unsupported RO compact
flag.
This will reject log replay no matter if we have dirty log or not, with
the following message:
BTRFS info (device dm-1): disabling log replay due to unsupported ro compat features
Cc: stable@vger.kernel.org #4.9+
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/disk-io.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index fe309db9f5ff..d06f1a176b5b 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3655,6 +3655,14 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
err = -EINVAL;
goto fail_alloc;
}
+ /*
+ * We have unsupported RO compat features, although RO mounted, we
+ * should any metadata write, including the log replay.
+ * Or we can screw up whatever the new feature requires.
+ */
+ if (features)
+ btrfs_set_and_info(fs_info, NOLOGREPLAY,
+ "disabling log replay due to unsupported ro compat features");
if (sectorsize < PAGE_SIZE) {
struct btrfs_subpage_info *subpage_info;
--
2.36.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] btrfs: reject log replay if there is unsupported RO flag
2022-06-07 2:31 [PATCH] btrfs: reject log replay if there is unsupported RO flag Qu Wenruo
@ 2022-06-07 9:49 ` Filipe Manana
2022-06-07 10:23 ` Qu Wenruo
0 siblings, 1 reply; 3+ messages in thread
From: Filipe Manana @ 2022-06-07 9:49 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, stable
On Tue, Jun 07, 2022 at 10:31:46AM +0800, Qu Wenruo wrote:
> [BUG]
> If we have a btrfs image with dirty log, along with an unsupported RO
> compatible flag:
>
> log_root 30474240
> ...
> compat_flags 0x0
> compat_ro_flags 0x40000003
> ( FREE_SPACE_TREE |
> FREE_SPACE_TREE_VALID |
> unknown flag: 0x40000000 )
>
> Then even if we can only mount it RO, we will still cause metadata
> update for log replay:
>
> BTRFS info (device dm-1): flagging fs with big metadata feature
> BTRFS info (device dm-1): using free space tree
> BTRFS info (device dm-1): has skinny extents
> BTRFS info (device dm-1): start tree-log replay
>
> This is definitely against RO compact flag requirement.
>
> [CAUSE]
> RO compact flag only forces us to do RO mount, but we will still do log
> replay for plain RO mount.
>
> Thus this will result us to do log replay and update metadata.
>
> This can be very problematic for new RO compat flag, for example older
> kernel can not understand v2 cache, and if we allow metadata update on
> RO mount and invalidate/corrupt v2 cache.
>
> [FIX]
> Just set the nologreplay flag if there is any unsupported RO compact
> flag.
>
> This will reject log replay no matter if we have dirty log or not, with
> the following message:
>
> BTRFS info (device dm-1): disabling log replay due to unsupported ro compat features
>
> Cc: stable@vger.kernel.org #4.9+
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> fs/btrfs/disk-io.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index fe309db9f5ff..d06f1a176b5b 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -3655,6 +3655,14 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
> err = -EINVAL;
> goto fail_alloc;
> }
> + /*
> + * We have unsupported RO compat features, although RO mounted, we
> + * should any metadata write, including the log replay.
> + * Or we can screw up whatever the new feature requires.
> + */
> + if (features)
> + btrfs_set_and_info(fs_info, NOLOGREPLAY,
> + "disabling log replay due to unsupported ro compat features");
Well, this might be surprising for users.
On mount, it's expected that everything that was fsynced is available.
Yes, there's a message printed informing the logs were not replayed,
but this allows for applications to read stale data.
I think just failing the mount and printing a message telling that the
fs needs to be explicitly mounted with -o nologreplay is less prone to
having stale data being read and used.
Thanks.
>
> if (sectorsize < PAGE_SIZE) {
> struct btrfs_subpage_info *subpage_info;
> --
> 2.36.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] btrfs: reject log replay if there is unsupported RO flag
2022-06-07 9:49 ` Filipe Manana
@ 2022-06-07 10:23 ` Qu Wenruo
0 siblings, 0 replies; 3+ messages in thread
From: Qu Wenruo @ 2022-06-07 10:23 UTC (permalink / raw)
To: Filipe Manana, Qu Wenruo; +Cc: linux-btrfs, stable
On 2022/6/7 17:49, Filipe Manana wrote:
> On Tue, Jun 07, 2022 at 10:31:46AM +0800, Qu Wenruo wrote:
>> [BUG]
>> If we have a btrfs image with dirty log, along with an unsupported RO
>> compatible flag:
>>
>> log_root 30474240
>> ...
>> compat_flags 0x0
>> compat_ro_flags 0x40000003
>> ( FREE_SPACE_TREE |
>> FREE_SPACE_TREE_VALID |
>> unknown flag: 0x40000000 )
>>
>> Then even if we can only mount it RO, we will still cause metadata
>> update for log replay:
>>
>> BTRFS info (device dm-1): flagging fs with big metadata feature
>> BTRFS info (device dm-1): using free space tree
>> BTRFS info (device dm-1): has skinny extents
>> BTRFS info (device dm-1): start tree-log replay
>>
>> This is definitely against RO compact flag requirement.
>>
>> [CAUSE]
>> RO compact flag only forces us to do RO mount, but we will still do log
>> replay for plain RO mount.
>>
>> Thus this will result us to do log replay and update metadata.
>>
>> This can be very problematic for new RO compat flag, for example older
>> kernel can not understand v2 cache, and if we allow metadata update on
>> RO mount and invalidate/corrupt v2 cache.
>>
>> [FIX]
>> Just set the nologreplay flag if there is any unsupported RO compact
>> flag.
>>
>> This will reject log replay no matter if we have dirty log or not, with
>> the following message:
>>
>> BTRFS info (device dm-1): disabling log replay due to unsupported ro compat features
>>
>> Cc: stable@vger.kernel.org #4.9+
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> fs/btrfs/disk-io.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index fe309db9f5ff..d06f1a176b5b 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -3655,6 +3655,14 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
>> err = -EINVAL;
>> goto fail_alloc;
>> }
>> + /*
>> + * We have unsupported RO compat features, although RO mounted, we
>> + * should any metadata write, including the log replay.
>> + * Or we can screw up whatever the new feature requires.
>> + */
>> + if (features)
>> + btrfs_set_and_info(fs_info, NOLOGREPLAY,
>> + "disabling log replay due to unsupported ro compat features");
>
> Well, this might be surprising for users.
>
> On mount, it's expected that everything that was fsynced is available.
> Yes, there's a message printed informing the logs were not replayed,
> but this allows for applications to read stale data.
>
> I think just failing the mount and printing a message telling that the
> fs needs to be explicitly mounted with -o nologreplay is less prone to
> having stale data being read and used.
OK, I'm fine with either method, as long as we reject the log when there
is unsupported RO compat feature.
Thanks,
Qu
>
> Thanks.
>
>>
>> if (sectorsize < PAGE_SIZE) {
>> struct btrfs_subpage_info *subpage_info;
>> --
>> 2.36.1
>>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-06-07 10:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-07 2:31 [PATCH] btrfs: reject log replay if there is unsupported RO flag Qu Wenruo
2022-06-07 9:49 ` Filipe Manana
2022-06-07 10:23 ` Qu Wenruo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.