7.3-stable review patch. If anyone has any objections, please let me know. ------------------ From: Darrick J. Wong commit aafe21c23ad8263251085291019d8860ddc51da9 upstream. LOLLM noticed that online repair of a broken symlink file could fail unnecessarily if a local-format symlink target isn't null terminated. The ondisk target isn't required to be null terminated, but repair enforces that anyway because it uses the validator for the incore symlink target. (The incore buffer is always null-terminated). Fix this by reverting the changes to xfs_symlink_shortform_verify and adding an ondisk-specific helper in inode_repair.c. Cc: stable@vger.kernel.org # v6.8 Fixes: e744cef2060559 ("xfs: zap broken inode forks") Signed-off-by: Darrick J. Wong Assisted-by: LOLLM # finding obvious bugs Reviewed-by: Christoph Hellwig Reviewed-by: Dave Chinner Signed-off-by: Carlos Maiolino Signed-off-by: Greg Kroah-Hartman --- fs/xfs/libxfs/xfs_inode_fork.c | 4 +--- fs/xfs/libxfs/xfs_symlink_remote.c | 8 ++++++-- fs/xfs/libxfs/xfs_symlink_remote.h | 2 +- fs/xfs/scrub/inode_repair.c | 26 +++++++++++++++++++++++++- 4 files changed, 33 insertions(+), 7 deletions(-) --- a/fs/xfs/libxfs/xfs_inode_fork.c +++ b/fs/xfs/libxfs/xfs_inode_fork.c @@ -683,9 +683,7 @@ xfs_ifork_verify_local_data( break; } case S_IFLNK: { - struct xfs_ifork *ifp = xfs_ifork_ptr(ip, XFS_DATA_FORK); - - fa = xfs_symlink_shortform_verify(ifp->if_data, ifp->if_bytes); + fa = xfs_symlink_shortform_verify(ip); break; } default: --- a/fs/xfs/libxfs/xfs_symlink_remote.c +++ b/fs/xfs/libxfs/xfs_symlink_remote.c @@ -208,11 +208,15 @@ xfs_symlink_local_to_remote( */ xfs_failaddr_t xfs_symlink_shortform_verify( - void *sfp, - int64_t size) + struct xfs_inode *ip) { + struct xfs_ifork *ifp = xfs_ifork_ptr(ip, XFS_DATA_FORK); + char *sfp = (char *)ifp->if_data; + int size = ifp->if_bytes; char *endp = sfp + size; + ASSERT(ifp->if_format == XFS_DINODE_FMT_LOCAL); + /* * Zero length symlinks should never occur in memory as they are * never allowed to exist on disk. --- a/fs/xfs/libxfs/xfs_symlink_remote.h +++ b/fs/xfs/libxfs/xfs_symlink_remote.h @@ -18,7 +18,7 @@ bool xfs_symlink_hdr_ok(xfs_ino_t ino, u void xfs_symlink_local_to_remote(struct xfs_trans *tp, struct xfs_buf *bp, struct xfs_inode *ip, struct xfs_ifork *ifp, void *priv); -xfs_failaddr_t xfs_symlink_shortform_verify(void *sfp, int64_t size); +xfs_failaddr_t xfs_symlink_shortform_verify(struct xfs_inode *ip); int xfs_symlink_remote_read(struct xfs_inode *ip, char *link); int xfs_symlink_write_target(struct xfs_trans *tp, struct xfs_inode *ip, xfs_ino_t owner, const char *target_path, int pathlen, --- a/fs/xfs/scrub/inode_repair.c +++ b/fs/xfs/scrub/inode_repair.c @@ -1024,6 +1024,30 @@ xrep_dinode_bad_metabt_fork( return false; } +static xfs_failaddr_t +xrep_symlink_shortform_verify( + void *sfp, + int64_t size) +{ + /* + * Zero length symlinks should never occur in memory as they are + * never allowed to exist on disk. + */ + if (!size) + return __this_address; + + /* No negative sizes or overly long symlink targets. */ + if (size < 0 || size > XFS_SYMLINK_MAXLEN) + return __this_address; + + /* No NULLs in the target either. */ + if (memchr(sfp, 0, size)) + return __this_address; + + /* ondisk symlink target isn't null terminated, unlike incore */ + return NULL; +} + /* * Check the data fork for things that will fail the ifork verifiers or the * ifork formatters. @@ -1099,7 +1123,7 @@ xrep_dinode_check_dfork( return true; /* symlink structure must pass verification. */ if (S_ISLNK(mode) && - xfs_symlink_shortform_verify(dfork_ptr, data_size) != NULL) + xrep_symlink_shortform_verify(dfork_ptr, data_size) != NULL) return true; break; case XFS_DINODE_FMT_EXTENTS: