diff options
author | Darrick J. Wong <djwong@kernel.org> | 2021-12-08 16:41:51 -0800 |
---|---|---|
committer | Darrick J. Wong <djwong@kernel.org> | 2021-12-15 17:29:32 -0800 |
commit | e659750c94e5a1c7b9398396e47ee00749d8973e (patch) | |
tree | a3df92f3481bd0469de71e546bfd02025a852a3d | |
parent | 90d15e3e9dcd303fbb36e9a54e7c36457ff9a254 (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.c | 11 | ||||
-rw-r--r-- | fs/xfs/xfs_iops.c | 36 | ||||
-rw-r--r-- | fs/xfs/xfs_symlink.c | 29 |
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); |