* [PATCH v2] xfs: fix the symbolic link assert in xfs_ifree [not found] <20130612182818.363497890@.sgi.com> @ 2013-06-12 18:28 ` Mark Tinguely 2013-06-13 2:37 ` Dave Chinner 0 siblings, 1 reply; 2+ messages in thread From: Mark Tinguely @ 2013-06-12 18:28 UTC (permalink / raw) To: xfs [-- Attachment #1: v2-xfs-fix-assert-in-xfs_ifree.patch --] [-- Type: text/plain, Size: 2922 bytes --] 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 <tinguely@sgi.com> --- 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 =================================================================== --- a/fs/xfs/xfs_symlink.c +++ b/fs/xfs/xfs_symlink.c @@ -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)); +} Index: b/fs/xfs/xfs_symlink.h =================================================================== --- a/fs/xfs/xfs_symlink.h +++ b/fs/xfs/xfs_symlink.h @@ -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 */ Index: b/fs/xfs/xfs_vnodeops.c =================================================================== --- a/fs/xfs/xfs_vnodeops.c +++ b/fs/xfs/xfs_vnodeops.c @@ -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 xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH v2] xfs: fix the symbolic link assert in xfs_ifree 2013-06-12 18:28 ` [PATCH v2] xfs: fix the symbolic link assert in xfs_ifree Mark Tinguely @ 2013-06-13 2:37 ` Dave Chinner 0 siblings, 0 replies; 2+ messages in thread From: Dave Chinner @ 2013-06-13 2:37 UTC (permalink / raw) To: Mark Tinguely; +Cc: xfs On Wed, Jun 12, 2013 at 01:28:19PM -0500, Mark Tinguely wrote: > 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 <tinguely@sgi.com> > --- > 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 > =================================================================== > --- a/fs/xfs/xfs_symlink.c > +++ b/fs/xfs/xfs_symlink.c > @@ -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) > +{ > + 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. > + /* > + * 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. > +} > Index: b/fs/xfs/xfs_symlink.h > =================================================================== > --- a/fs/xfs/xfs_symlink.h > +++ b/fs/xfs/xfs_symlink.h > @@ -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. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2013-06-13 2:37 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20130612182818.363497890@.sgi.com>
2013-06-12 18:28 ` [PATCH v2] xfs: fix the symbolic link assert in xfs_ifree Mark Tinguely
2013-06-13 2:37 ` Dave Chinner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox