xfs: read only mounts with fsopen mount API are busted
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: Dave Chinner <dchinner@redhat.com>
Fixes: 73e5fff98b
("xfs: switch to use the new mount-api")
cc: stable@vger.kernel.org
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
This commit is contained in:
parent
6613476e22
commit
d8d222e09d
|
@ -1496,6 +1496,18 @@ xfs_fs_fill_super(
|
||||||
|
|
||||||
mp->m_super = sb;
|
mp->m_super = sb;
|
||||||
|
|
||||||
|
/*
|
||||||
|
* Copy VFS mount flags from the context now that all parameter parsing
|
||||||
|
* is guaranteed to have been completed by either the old mount API or
|
||||||
|
* the newer fsopen/fsconfig API.
|
||||||
|
*/
|
||||||
|
if (fc->sb_flags & SB_RDONLY)
|
||||||
|
set_bit(XFS_OPSTATE_READONLY, &mp->m_opstate);
|
||||||
|
if (fc->sb_flags & SB_DIRSYNC)
|
||||||
|
mp->m_features |= XFS_FEAT_DIRSYNC;
|
||||||
|
if (fc->sb_flags & SB_SYNCHRONOUS)
|
||||||
|
mp->m_features |= XFS_FEAT_WSYNC;
|
||||||
|
|
||||||
error = xfs_fs_validate_params(mp);
|
error = xfs_fs_validate_params(mp);
|
||||||
if (error)
|
if (error)
|
||||||
return error;
|
return error;
|
||||||
|
@ -1965,6 +1977,11 @@ static const struct fs_context_operations xfs_context_ops = {
|
||||||
.free = xfs_fs_free,
|
.free = xfs_fs_free,
|
||||||
};
|
};
|
||||||
|
|
||||||
|
/*
|
||||||
|
* WARNING: do not initialise any parameters in this function that depend on
|
||||||
|
* mount option parsing having already been performed as this can be called from
|
||||||
|
* fsopen() before any parameters have been set.
|
||||||
|
*/
|
||||||
static int xfs_init_fs_context(
|
static int xfs_init_fs_context(
|
||||||
struct fs_context *fc)
|
struct fs_context *fc)
|
||||||
{
|
{
|
||||||
|
@ -1996,16 +2013,6 @@ static int xfs_init_fs_context(
|
||||||
mp->m_logbsize = -1;
|
mp->m_logbsize = -1;
|
||||||
mp->m_allocsize_log = 16; /* 64k */
|
mp->m_allocsize_log = 16; /* 64k */
|
||||||
|
|
||||||
/*
|
|
||||||
* Copy binary VFS mount flags we are interested in.
|
|
||||||
*/
|
|
||||||
if (fc->sb_flags & SB_RDONLY)
|
|
||||||
set_bit(XFS_OPSTATE_READONLY, &mp->m_opstate);
|
|
||||||
if (fc->sb_flags & SB_DIRSYNC)
|
|
||||||
mp->m_features |= XFS_FEAT_DIRSYNC;
|
|
||||||
if (fc->sb_flags & SB_SYNCHRONOUS)
|
|
||||||
mp->m_features |= XFS_FEAT_WSYNC;
|
|
||||||
|
|
||||||
fc->s_fs_info = mp;
|
fc->s_fs_info = mp;
|
||||||
fc->ops = &xfs_context_ops;
|
fc->ops = &xfs_context_ops;
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue