Skip to content

Commit

Permalink
xfs: remove xfs_tosspages
Browse files Browse the repository at this point in the history
It's a buggy, unnecessary wrapper that is duplicating
truncate_pagecache_range().

When replacing the call in xfs_change_file_space(), also ensure that
the length being allocated/freed is always positive before making
any changes. These checks are done in the lower extent manipulation
functions, too, but we need to do them before any page cache
operations.

Reported-by: Andrew Dahl <adahl@sgi.com>
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-By: Andrew Dahl <adahl@sgi.com>
Signed-off-by: Ben Myers <bpm@sgi.com>
  • Loading branch information
Dave Chinner authored and Ben Myers committed Nov 14, 2012
1 parent de49768 commit f5b8911
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 21 deletions.
3 changes: 1 addition & 2 deletions fs/xfs/xfs_dfrag.c
Original file line number Diff line number Diff line change
Expand Up @@ -315,8 +315,7 @@ xfs_swap_extents(
* are safe. We don't really care if non-io related
* fields change.
*/

xfs_tosspages(ip, 0, -1, FI_REMAPF);
truncate_pagecache_range(VFS_I(ip), 0, -1);

tp = xfs_trans_alloc(mp, XFS_TRANS_SWAPEXT);
if ((error = xfs_trans_reserve(tp, 0,
Expand Down
12 changes: 0 additions & 12 deletions fs/xfs/xfs_fs_subr.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,6 @@
* note: all filemap functions return negative error codes. These
* need to be inverted before returning to the xfs core functions.
*/
void
xfs_tosspages(
xfs_inode_t *ip,
xfs_off_t first,
xfs_off_t last,
int fiopt)
{
/* can't toss partial tail pages, so mask them out */
last &= ~(PAGE_SIZE - 1);
truncate_inode_pages_range(VFS_I(ip)->i_mapping, first, last - 1);
}

int
xfs_flushinval_pages(
xfs_inode_t *ip,
Expand Down
30 changes: 25 additions & 5 deletions fs/xfs/xfs_vnodeops.c
Original file line number Diff line number Diff line change
Expand Up @@ -2118,7 +2118,7 @@ xfs_change_file_space(
xfs_fsize_t fsize;
int setprealloc;
xfs_off_t startoffset;
xfs_off_t llen;
xfs_off_t end;
xfs_trans_t *tp;
struct iattr iattr;
int prealloc_type;
Expand All @@ -2139,12 +2139,30 @@ xfs_change_file_space(
return XFS_ERROR(EINVAL);
}

llen = bf->l_len > 0 ? bf->l_len - 1 : bf->l_len;
/*
* length of <= 0 for resv/unresv/zero is invalid. length for
* alloc/free is ignored completely and we have no idea what userspace
* might have set it to, so set it to zero to allow range
* checks to pass.
*/
switch (cmd) {
case XFS_IOC_ZERO_RANGE:
case XFS_IOC_RESVSP:
case XFS_IOC_RESVSP64:
case XFS_IOC_UNRESVSP:
case XFS_IOC_UNRESVSP64:
if (bf->l_len <= 0)
return XFS_ERROR(EINVAL);
break;
default:
bf->l_len = 0;
break;
}

if (bf->l_start < 0 ||
bf->l_start > mp->m_super->s_maxbytes ||
bf->l_start + llen < 0 ||
bf->l_start + llen > mp->m_super->s_maxbytes)
bf->l_start + bf->l_len < 0 ||
bf->l_start + bf->l_len >= mp->m_super->s_maxbytes)
return XFS_ERROR(EINVAL);

bf->l_whence = 0;
Expand All @@ -2169,7 +2187,9 @@ xfs_change_file_space(
switch (cmd) {
case XFS_IOC_ZERO_RANGE:
prealloc_type |= XFS_BMAPI_CONVERT;
xfs_tosspages(ip, startoffset, startoffset + bf->l_len, 0);
end = round_down(startoffset + bf->l_len, PAGE_SIZE) - 1;
if (startoffset > end)
truncate_pagecache_range(VFS_I(ip), startoffset, end);
/* FALLTHRU */
case XFS_IOC_RESVSP:
case XFS_IOC_RESVSP64:
Expand Down
2 changes: 0 additions & 2 deletions fs/xfs/xfs_vnodeops.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ int xfs_attr_set(struct xfs_inode *dp, const unsigned char *name,
int xfs_attr_remove(struct xfs_inode *dp, const unsigned char *name, int flags);
int xfs_attr_list(struct xfs_inode *dp, char *buffer, int bufsize,
int flags, struct attrlist_cursor_kern *cursor);
void xfs_tosspages(struct xfs_inode *inode, xfs_off_t first,
xfs_off_t last, int fiopt);
int xfs_flushinval_pages(struct xfs_inode *ip, xfs_off_t first,
xfs_off_t last, int fiopt);
int xfs_flush_pages(struct xfs_inode *ip, xfs_off_t first,
Expand Down

0 comments on commit f5b8911

Please sign in to comment.