xfs: fix the symbolic link assert in xfs_ifree
PATCH v4 1/1 xfs: fix the symbolic link assert in xfs_ifree
a
Content-Disposition: inline; filename=v4-xfs-fix-assert-in-xfs_ifree.patch
Adding an extended attribute to a symbolic link can force that link to an remote extent. xfs_inactive() incorrectly assumes that any symbolic link small enough to be in the inode core is incore, resulting in the remote extent to not be removed. xfs_ifree() will assert on presence of this leaked remote extent.
---
v4:
changed pathlen to int.
print inode in hex
fixed the trace to be xfs_inactive_symlink in source and trace code.
fs/xfs/xfs_symlink.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++-- fs/xfs/xfs_symlink.h | 2 +- fs/xfs/xfs_trace.h | 1 + fs/xfs/xfs_vnodeops.c | 15 +++------------ 4 files changed, 51 insertions(+), 15 deletions(-)
fs/xfs/xfs_symlink.c +46 -2
@@ -585,7 +585,7 @@ xfs_symlink(/* * Free a symlink that has blocks associated with it. */ -int +STATIC intxfs_inactive_symlink_rmt (xfs_inode_t *ip,xfs_trans_t **tpp)
@@ -606,7 +606,7 @@ xfs_inactive_symlink_rmt( tp = *tpp; mp = ip->i_mount; -ASSERT (ip->i_d.di_size >XFS_IFORK_DSIZE (ip)); +ASSERT (ip->i_df.if_flags &XFS_IFEXTENTS );/* * We're freeing a symlink that has some * blocks allocated to it. Free the
@@ -720,3 +720,47 @@ xfs_inactive_symlink_rmt( error0:return error; } + +/* +* xfs_inactive_symlink - free a symlink +*/ +int +xfs_inactive_symlink ( +struct xfs_inode *ip, +struct xfs_trans **tp) +{ +struct xfs_mount *mp = ip->i_mount; +int pathlen; + +trace_xfs_inactive_symlink (ip); + +ASSERT (xfs_isilocked (ip,XFS_ILOCK_EXCL )); + +if (XFS_FORCED_SHUTDOWN (mp)) +return XFS_ERROR (EIO ); + +/* +* Zero length symlinks _can_ exist. +*/ + pathlen = (int )ip->i_d.di_size; +if (!pathlen) +return 0 ; + +if (pathlen <0 || pathlen >MAXPATHLEN ) { +xfs_alert (mp,"%s: inode (%llx) bad symlink length (%d)" , + __func__, (unsigned long long )ip->i_ino, pathlen); +ASSERT (0 ); +return XFS_ERROR (EFSCORRUPTED ); + } + +if (ip->i_df.if_flags &XFS_IFINLINE ) { +if (ip->i_df.if_bytes >0 ) +xfs_idata_realloc (ip, -(ip->i_df.if_bytes), +XFS_DATA_FORK ); +ASSERT (ip->i_df.if_bytes ==0 ); +return 0 ; + } + +/* remove the remote symlink */ +return xfs_inactive_symlink_rmt (ip, tp); +}
fs/xfs/xfs_symlink.h +1 -1
@@ -60,7 +60,7 @@ extern const struct xfs_buf_ops xfs_symlint xfs_symlink (struct xfs_inode *dp,struct xfs_name *link_name,const char *target_path,umode_t mode,struct xfs_inode **ipp);int xfs_readlink (struct xfs_inode *ip,char *link); -int xfs_inactive_symlink_rmt (struct xfs_inode *ip,struct xfs_trans **tpp); +int xfs_inactive_symlink (struct xfs_inode *ip,struct xfs_trans **tpp);#endif /* __KERNEL__ */ #endif /* __XFS_SYMLINK_H */
fs/xfs/xfs_trace.h +1 -0
@@ -571,6 +571,7 @@ DEFINE_INODE_EVENT(xfs_iget_miss);DEFINE_INODE_EVENT (xfs_getattr);DEFINE_INODE_EVENT (xfs_setattr);DEFINE_INODE_EVENT (xfs_readlink); +DEFINE_INODE_EVENT (xfs_inactive_symlink);DEFINE_INODE_EVENT (xfs_alloc_file_space);DEFINE_INODE_EVENT (xfs_free_file_space);DEFINE_INODE_EVENT (xfs_readdir);
fs/xfs/xfs_vnodeops.c +3 -12
@@ -322,18 +322,9 @@ xfs_inactive(xfs_trans_ijoin (tp, ip,0 );if (S_ISLNK (ip->i_d.di_mode)) { -/* -* Zero length symlinks _can_ exist. -*/ -if (ip->i_d.di_size >XFS_IFORK_DSIZE (ip)) { - error =xfs_inactive_symlink_rmt (ip, &tp); -if (error) - goto out_cancel; - }else if (ip->i_df.if_bytes >0 ) { -xfs_idata_realloc (ip, -(ip->i_df.if_bytes), -XFS_DATA_FORK ); -ASSERT (ip->i_df.if_bytes ==0 ); - } + error =xfs_inactive_symlink (ip, &tp); +if (error) + goto out_cancel; }else if (truncate) { ip->i_d.di_size =0 ;xfs_trans_log_inode (tp, ip,XFS_ILOG_CORE );
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 7 lines · Mark Tinguely
Adding an extended attribute to a symbolic link can force that link to an remote extent.
xfs_inactive()incorrectly assumes that any symbolic link small enough to be in the inode core is incore, resulting in the remote extent to not be removed.xfs_ifree()will assert on presence of this leaked remote extent.Signed-off-by Mark Tinguely <[email protected]>
Looks good.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 10 lines · Mark Tinguely
a
Content-Disposition: inline; filename=v4-xfs-fix-assert-in-xfs_ifree.patchAdding an extended attribute to a symbolic link can force that link to an remote extent.
xfs_inactive()incorrectly assumes that any symbolic link small enough to be in the inode core is incore, resulting in the remote extent to not be removed.xfs_ifree()will assert on presence of this leaked remote extent.Signed-off-by Mark Tinguely <[email protected]>
Looks good.
One minor, minor thing:
fs/xfs/xfs_symlink.c xfs_inactive_symlink_rmt( · line 750
Quoted 2 lines
fs/xfs/xfs_symlink.c +2
-0+if (pathlen <0 || pathlen >MAXPATHLEN ) { + xfs_alert(mp, "%s: inode (%llx) bad symlink length (%d)",
(0x%llx)
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs