On 26.03.19 г. 12:49 ч., fdman...@kernel.org wrote:
> From: Filipe Manana <fdman...@suse.com>
> 
> Whan a filesystem is mounted with the nologreplay mount option, which
> requires it to be mounted in RO mode as well, we can not allow discard on
> free space inside block groups, because log trees refer to extents that
> are not pinned in a block group's free space cache (pinning the extents is
> precisely the first phase of replaying a log tree).
> 
> So do not allow the fitrim ioctl to do anything when the filesystem is
> mounted with the nologreplay option, because later it can be mounted RW
> without that option, which causes log replay to happen and result in
> either a failure to replay the log trees (leading to a mount failure), a
> crash or some silent corruption.
> 
> Reported-by: Darrick J. Wong <darrick.w...@oracle.com>
> Signed-off-by: Filipe Manana <fdman...@suse.com>

Does it make sense to make the check a bit more specific and only return
EROFS when NOLOGREPLAY and the log tree has non-null generation?

In any case:

Reviewed-by: Nikolay Borisov <nbori...@suse.com>

> ---
>  fs/btrfs/ioctl.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 494f0f10d70e..01808934d21f 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -501,6 +501,16 @@ static noinline int btrfs_ioctl_fitrim(struct file 
> *file, void __user *arg)
>       if (!capable(CAP_SYS_ADMIN))
>               return -EPERM;
>  
> +     /*
> +      * If the fs is mounted with nologreplay, which requires it to be
> +      * mounted in RO mode as well, we can not allow discard on free space
> +      * inside block groups, because log trees refer to extents that are not
> +      * pinned in a block group's free space cache (pinning the extents is
> +      * precisely the first phase of replaying a log tree).
> +      */
> +     if (btrfs_test_opt(fs_info, NOLOGREPLAY))
> +             return -EROFS;
> +
>       rcu_read_lock();
>       list_for_each_entry_rcu(device, &fs_info->fs_devices->devices,
>                               dev_list) {
> 

Reply via email to