• Dave Chinner's avatar
    xfs: read only mounts with fsopen mount API are busted · d8d222e0
    Dave Chinner authored
    Recently xfs/513 started failing on my test machines testing "-o
    ro,norecovery" mount options. This was being emitted in dmesg:
    
    [ 9906.932724] XFS (pmem0): no-recovery mounts must be read-only.
    
    Turns out, readonly mounts with the fsopen()/fsconfig() mount API
    have been busted since day zero. It's only taken 5 years for debian
    unstable to start using this "new" mount API, and shortly after this
    I noticed xfs/513 had started to fail as per above.
    
    The syscall trace is:
    
    fsopen("xfs", FSOPEN_CLOEXEC)           = 3
    mount_setattr(-1, NULL, 0, NULL, 0)     = -1 EINVAL (Invalid argument)
    .....
    fsconfig(3, FSCONFIG_SET_STRING, "source", "/dev/pmem0", 0) = 0
    fsconfig(3, FSCONFIG_SET_FLAG, "ro", NULL, 0) = 0
    fsconfig(3, FSCONFIG_SET_FLAG, "norecovery", NULL, 0) = 0
    fsconfig(3, FSCONFIG_CMD_CREATE, NULL, NULL, 0) = -1 EINVAL (Invalid argument)
    close(3)                                = 0
    
    Showing that the actual mount instantiation (FSCONFIG_CMD_CREATE) is
    what threw out the error.
    
    During mount instantiation, we call xfs_fs_validate_params() which
    does:
    
            /* No recovery flag requires a read-only mount */
            if (xfs_has_norecovery(mp) && !xfs_is_readonly(mp)) {
                    xfs_warn(mp, "no-recovery mounts must be read-only.");
                    return -EINVAL;
            }
    
    and xfs_is_readonly() checks internal mount flags for read only
    state. This state is set in xfs_init_fs_context() from the
    context superblock flag state:
    
            /*
             * Copy binary VFS mount flags we are interested in.
             */
            if (fc->sb_flags & SB_RDONLY)
                    set_bit(XFS_OPSTATE_READONLY, &mp->m_opstate);
    
    With the old mount API, all of the VFS specific superblock flags
    had already been parsed and set before xfs_init_fs_context() is
    called, so this all works fine.
    
    However, in the brave new fsopen/fsconfig world,
    xfs_init_fs_context() is called from fsopen() context, before any
    VFS superblock have been set or parsed. Hence if we use fsopen(),
    the internal XFS readonly state is *never set*. Hence anything that
    depends on xfs_is_readonly() actually returning true for read only
    mounts is broken if fsopen() has been used to mount the filesystem.
    
    Fix this by moving this internal state initialisation to
    xfs_fs_fill_super() before we attempt to validate the parameters
    that have been set prior to the FSCONFIG_CMD_CREATE call being made.
    Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
    Fixes: 73e5fff9 ("xfs: switch to use the new mount-api")
    cc: stable@vger.kernel.org
    Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
    Signed-off-by: default avatarChandan Babu R <chandanbabu@kernel.org>
    d8d222e0
xfs_super.c 61.8 KB