Skip to content

Commit

Permalink
[XFS] Fix race condition in xfs_write().
Browse files Browse the repository at this point in the history
This change addresses a race in xfs_write() where, for direct I/O, the
flags need_i_mutex and need_flush are setup before the iolock is acquired.
The logic used to setup the flags may change between setting the flags and
acquiring the iolock resulting in these flags having incorrect values. For
example, if a file is not currently cached then need_i_mutex is set to
zero and then if the file is cached before the iolock is acquired we will
fail to do the flushinval before the direct write.

The flush (and also the call to xfs_zero_eof()) need to be done with the
iolock held exclusive so we need to acquire the iolock before checking for
cached data (or if the write begins after eof) to prevent this state from
changing. For direct I/O I've chosen to always acquire the iolock in
shared mode initially and if there is a need to promote it then drop it
and reacquire it.

There's also some other tidy-ups including removing the O_APPEND offset
adjustment since that work is done in generic_write_checks() (and we don't
use offset as an input parameter anywhere).

SGI-PV: 962170
SGI-Modid: xfs-linux-melb:xfs-kern:28319a

Signed-off-by: Lachlan McIlroy <lachlan@sgi.com>
Signed-off-by: David Chinner <dgc@sgi.com>
Signed-off-by: Tim Shimmin <tes@sgi.com>
  • Loading branch information
Lachlan McIlroy authored and Tim Shimmin committed May 8, 2007
1 parent e6d2942 commit 2a32963
Showing 1 changed file with 32 additions and 29 deletions.
61 changes: 32 additions & 29 deletions fs/xfs/linux-2.6/xfs_lrw.c
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,7 @@ xfs_write(
bhv_vrwlock_t locktype;
size_t ocount = 0, count;
loff_t pos;
int need_i_mutex = 1, need_flush = 0;
int need_i_mutex;

XFS_STATS_INC(xs_write_calls);

Expand Down Expand Up @@ -689,39 +689,20 @@ xfs_write(
if (XFS_FORCED_SHUTDOWN(mp))
return -EIO;

if (ioflags & IO_ISDIRECT) {
xfs_buftarg_t *target =
(xip->i_d.di_flags & XFS_DIFLAG_REALTIME) ?
mp->m_rtdev_targp : mp->m_ddev_targp;

if ((pos & target->bt_smask) || (count & target->bt_smask))
return XFS_ERROR(-EINVAL);

if (!VN_CACHED(vp) && pos < i_size_read(inode))
need_i_mutex = 0;

if (VN_CACHED(vp))
need_flush = 1;
}

relock:
if (need_i_mutex) {
if (ioflags & IO_ISDIRECT) {
iolock = XFS_IOLOCK_SHARED;
locktype = VRWLOCK_WRITE_DIRECT;
need_i_mutex = 0;
} else {
iolock = XFS_IOLOCK_EXCL;
locktype = VRWLOCK_WRITE;

need_i_mutex = 1;
mutex_lock(&inode->i_mutex);
} else {
iolock = XFS_IOLOCK_SHARED;
locktype = VRWLOCK_WRITE_DIRECT;
}

xfs_ilock(xip, XFS_ILOCK_EXCL|iolock);

isize = i_size_read(inode);

if (file->f_flags & O_APPEND)
*offset = isize;

start:
error = -generic_write_checks(file, &pos, &count,
S_ISBLK(inode->i_mode));
Expand All @@ -730,6 +711,29 @@ xfs_write(
goto out_unlock_mutex;
}

isize = i_size_read(inode);

if (ioflags & IO_ISDIRECT) {
xfs_buftarg_t *target =
(xip->i_d.di_flags & XFS_DIFLAG_REALTIME) ?
mp->m_rtdev_targp : mp->m_ddev_targp;

if ((pos & target->bt_smask) || (count & target->bt_smask)) {
xfs_iunlock(xip, XFS_ILOCK_EXCL|iolock);
return XFS_ERROR(-EINVAL);
}

if (!need_i_mutex && (VN_CACHED(vp) || pos > isize)) {
xfs_iunlock(xip, XFS_ILOCK_EXCL|iolock);
iolock = XFS_IOLOCK_EXCL;
locktype = VRWLOCK_WRITE;
need_i_mutex = 1;
mutex_lock(&inode->i_mutex);
xfs_ilock(xip, XFS_ILOCK_EXCL|iolock);
goto start;
}
}

new_size = pos + count;
if (new_size > isize)
io->io_new_size = new_size;
Expand Down Expand Up @@ -761,7 +765,6 @@ xfs_write(
* what allows the size to change in the first place.
*/
if ((file->f_flags & O_APPEND) && savedsize != isize) {
pos = isize = xip->i_d.di_size;
goto start;
}
}
Expand Down Expand Up @@ -815,7 +818,8 @@ xfs_write(
current->backing_dev_info = mapping->backing_dev_info;

if ((ioflags & IO_ISDIRECT)) {
if (need_flush) {
if (VN_CACHED(vp)) {
WARN_ON(need_i_mutex == 0);
xfs_inval_cached_trace(io, pos, -1,
ctooff(offtoct(pos)), -1);
error = bhv_vop_flushinval_pages(vp, ctooff(offtoct(pos)),
Expand Down Expand Up @@ -849,7 +853,6 @@ xfs_write(
pos += ret;
count -= ret;

need_i_mutex = 1;
ioflags &= ~IO_ISDIRECT;
xfs_iunlock(xip, iolock);
goto relock;
Expand Down

0 comments on commit 2a32963

Please sign in to comment.