summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDarrick J. Wong <djwong@kernel.org>2021-12-08 16:41:51 -0800
committerDarrick J. Wong <djwong@kernel.org>2021-12-15 17:29:32 -0800
commite659750c94e5a1c7b9398396e47ee00749d8973e (patch)
treea3df92f3481bd0469de71e546bfd02025a852a3d
parent90d15e3e9dcd303fbb36e9a54e7c36457ff9a254 (diff)
xfs: merge the two symlink inode_operations
It's really awkward that we maintain two separate VFS inode operations structures and functions for handling symlinks, because that means online repair potentially has to switch iops after fixing a symlink. Merge them into one to obviate the need for this. Furthermore, this fixes the issue of the VFS readlink functions retaining a pointer to if_data after the inode has been recycled; protects XFS against the VFS altering the contents of internal metadata buffers (if_data); and shuts down complaints about us accessing the inode data fork without taking any locks. In the past, we've skirted these issues because symlink targets are basically immutable, but this won't be the case with online repair. Signed-off-by: Darrick J. Wong <djwong@kernel.org>
-rw-r--r--fs/xfs/scrub/symlink_repair.c11
-rw-r--r--fs/xfs/xfs_iops.c36
-rw-r--r--fs/xfs/xfs_symlink.c29
3 files changed, 22 insertions, 54 deletions
diff --git a/fs/xfs/scrub/symlink_repair.c b/fs/xfs/scrub/symlink_repair.c
index 513e68728e03..e9ecf83216c2 100644
--- a/fs/xfs/scrub/symlink_repair.c
+++ b/fs/xfs/scrub/symlink_repair.c
@@ -225,16 +225,7 @@ xrep_symlink_reinitialize(
return error;
/* Finish up any block mapping activities. */
- error = xfs_defer_finish(&sc->tp);
- if (error)
- return error;
-
- /*
- * Set the correct VFS inode operations. We hold all inode locks, so
- * this should be safe against other threads.
- */
- xfs_setup_iops(sc->ip);
- return 0;
+ return xfs_defer_finish(&sc->tp);
}
/* Repair a symbolic link. */
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index e4ccef86fe28..1146a9985213 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -535,29 +535,6 @@ xfs_vn_get_link(
return ERR_PTR(error);
}
-STATIC const char *
-xfs_vn_get_link_inline(
- struct dentry *dentry,
- struct inode *inode,
- struct delayed_call *done)
-{
- struct xfs_inode *ip = XFS_I(inode);
- char *link;
-
- ASSERT(ip->i_df.if_format == XFS_DINODE_FMT_LOCAL);
-
- /*
- * The VFS crashes on a NULL pointer, so return -EFSCORRUPTED if
- * if_data is junk.
- */
- link = ip->i_df.if_u1.if_data;
- if (XFS_IS_CORRUPT(ip->i_mount, !link)) {
- xfs_inode_mark_sick(ip, XFS_SICK_INO_SYMLINK);
- return ERR_PTR(-EFSCORRUPTED);
- }
- return link;
-}
-
static uint32_t
xfs_stat_blksize(
struct xfs_inode *ip)
@@ -1290,14 +1267,6 @@ static const struct inode_operations xfs_symlink_inode_operations = {
.update_time = xfs_vn_update_time,
};
-static const struct inode_operations xfs_inline_symlink_inode_operations = {
- .get_link = xfs_vn_get_link_inline,
- .getattr = xfs_vn_getattr,
- .setattr = xfs_vn_setattr,
- .listxattr = xfs_vn_listxattr,
- .update_time = xfs_vn_update_time,
-};
-
/* Figure out if this file actually supports DAX. */
static bool
xfs_inode_supports_dax(
@@ -1469,10 +1438,7 @@ xfs_setup_iops(
inode->i_fop = &xfs_dir_file_operations;
break;
case S_IFLNK:
- if (ip->i_df.if_format == XFS_DINODE_FMT_LOCAL)
- inode->i_op = &xfs_inline_symlink_inode_operations;
- else
- inode->i_op = &xfs_symlink_inode_operations;
+ inode->i_op = &xfs_symlink_inode_operations;
break;
default:
inode->i_op = &xfs_inode_operations;
diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index e41c8e8fcf9e..4ad97295bdfc 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -24,22 +24,21 @@
#include "xfs_ialloc.h"
#include "xfs_health.h"
#include "xfs_symlink_remote.h"
+#include "xfs_error.h"
/* ----- Kernel only functions below ----- */
int
xfs_readlink(
- struct xfs_inode *ip,
- char *link)
+ struct xfs_inode *ip,
+ char *link)
{
- struct xfs_mount *mp = ip->i_mount;
- xfs_fsize_t pathlen;
- int error = 0;
+ struct xfs_mount *mp = ip->i_mount;
+ xfs_fsize_t pathlen;
+ int error = 0;
trace_xfs_readlink(ip);
- ASSERT(ip->i_df.if_format != XFS_DINODE_FMT_LOCAL);
-
if (xfs_is_shutdown(mp))
return -EIO;
@@ -59,8 +58,20 @@ xfs_readlink(
goto out;
}
-
- error = xfs_symlink_remote_read(ip, link);
+ if (ip->i_df.if_format == XFS_DINODE_FMT_LOCAL) {
+ /*
+ * The VFS crashes on a NULL pointer, so return -EFSCORRUPTED
+ * if if_data is junk.
+ */
+ if (XFS_IS_CORRUPT(ip->i_mount, !ip->i_df.if_u1.if_data)) {
+ xfs_inode_mark_sick(ip, XFS_SICK_INO_SYMLINK);
+ return error;
+ }
+
+ memcpy(link, ip->i_df.if_u1.if_data, pathlen + 1);
+ } else {
+ error = xfs_symlink_remote_read(ip, link);
+ }
out:
xfs_iunlock(ip, XFS_ILOCK_SHARED);