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@xxxxxxxxxxxxxxx #4.9+
> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
> ---
> 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
>
