xfs: fix the symbolic link assert in xfs_ifree
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH v2 1/1 xfs: fix the symbolic link assert in xfs_ifree
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.
---
xfstest is coming.
fs/xfs/xfs_symlink.c | 24 +++++++++++++++++++++++- fs/xfs/xfs_symlink.h | 1 + fs/xfs/xfs_vnodeops.c | 15 +++------------ 3 files changed, 27 insertions(+), 13 deletions(-)
fs/xfs/xfs_symlink.c +23 -1
@@ -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,25 @@ 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) +{ + +/* +* Zero length symlinks _can_ exist. +*/ +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 ; + }else +return (xfs_inactive_symlink_rmt (ip, tp)); +}
fs/xfs/xfs_symlink.h +1 -0
@@ -61,6 +61,7 @@ int xfs_symlink(struct xfs_inode *dp, st 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_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 32 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]>---
xfstest is coming.fs/xfs/xfs_symlink.c | 24 +++++++++++++++++++++++- fs/xfs/xfs_symlink.h | 1 + fs/xfs/xfs_vnodeops.c | 15 +++------------ 3 files changed, 27 insertions(+), 13 deletions(-)Index:
b/fs/xfs/xfs_symlink.c
===================================================================fs/xfs/xfs_symlink.c +1
-1@@ -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,25 @@ xfs_inactive_symlink_rmt( error0:return error; }
fs/xfs/xfs_symlink.c xfs_inactive_symlink_rmt( · line 732
Quoted 10 lines
fs/xfs/xfs_symlink.c +10
-0+ +/* +* xfs_inactive_symlink - free a symlink +*/ +int +xfs_inactive_symlink ( +struct xfs_inode *ip, +struct xfs_trans **tp) +{ +
Needs to:
- emit a tracepoint (trace_xfs_symlink_inactive(ip))
- assert the inode is locked appropriately
- check for a shutdown condition
- check for zero size and return success immediately
- check inode size is within bounds (0 < size < MAXPATHLEN)
i.e. all the same preamble that xfs_readlink() has.
fs/xfs/xfs_symlink.c xfs_inactive_symlink_rmt( · line 743
Quoted 11 lines
fs/xfs/xfs_symlink.c +11
-0+/* +* Zero length symlinks _can_ exist. +*/ +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 ; + }else +return (xfs_inactive_symlink_rmt (ip, tp));
No need for an else here, and no need for the () around the xfs_inactive_symlink_rmt() function call.
fs/xfs/xfs_symlink.h int xfs_symlink(struct xfs_inode *dp, st · line 64
Quoted 10 lines
fs/xfs/xfs_symlink.c +1
-0+}Index:
b/fs/xfs/xfs_symlink.h
===================================================================fs/xfs/xfs_symlink.h +1
-0@@ -61,6 +61,7 @@ int xfs_symlink(struct xfs_inode *dp, st 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);
xfs_inactive_symlink_rmt() is no longer exported, so it can be removed from the header and made STATIC.
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs