fix O_SYNC|O_APPEND syncing the wrong range on write()

Al Viro · Al Viro · commit d311d79de305 · 2014-02-09T15:18:09.000-05:00
It actually goes back to 2004 ([PATCH] Concurrent O_SYNC write support)
when sync_page_range() had been introduced; generic_file_write{,v}() correctly
synced
	pos_after_write - written .. pos_after_write - 1
but generic_file_aio_write() synced
	pos_before_write .. pos_before_write + written - 1
instead.  Which is not the same thing with O_APPEND, obviously.
A couple of years later correct variant had been killed off when
everything switched to use of generic_file_aio_write().

All users of generic_file_aio_write() are affected, and the same bug
has been copied into other instances of -&gt;aio_write().

The fix is trivial; the only subtle point is that generic_write_sync()
ought to be inlined to avoid calculations useless for the majority of
calls.

Signed-off-by: Al Viro &lt;viro@zeniv.linux.org.uk&gt;
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
@@ -2559,8 +2559,8 @@ cifs_writev(struct kiocb *iocb, const struct iovec *iov,
 	if (rc > 0) {
 		ssize_t err;
 
-		err = generic_write_sync(file, pos, rc);
-		if (err < 0 && rc > 0)
+		err = generic_write_sync(file, iocb->ki_pos - rc, rc);
+		if (err < 0)
 			rc = err;
 	}
 
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
@@ -152,7 +152,7 @@ ext4_file_dio_write(struct kiocb *iocb, const struct iovec *iov,
 	if (ret > 0) {
 		ssize_t err;
 
-		err = generic_write_sync(file, pos, ret);
+		err = generic_write_sync(file, iocb->ki_pos - ret, ret);
 		if (err < 0 && ret > 0)
 			ret = err;
 	}
diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
@@ -2134,7 +2134,7 @@ static ssize_t ntfs_file_aio_write(struct kiocb *iocb, const struct iovec *iov,
 	ret = ntfs_file_aio_write_nolock(iocb, iov, nr_segs, &iocb->ki_pos);
 	mutex_unlock(&inode->i_mutex);
 	if (ret > 0) {
-		int err = generic_write_sync(file, pos, ret);
+		int err = generic_write_sync(file, iocb->ki_pos - ret, ret);
 		if (err < 0)
 			ret = err;
 	}
diff --git a/fs/sync.c b/fs/sync.c
@@ -222,23 +222,6 @@ SYSCALL_DEFINE1(fdatasync, unsigned int, fd)
 	return do_fsync(fd, 1);
 }
 
-/**
- * generic_write_sync - perform syncing after a write if file / inode is sync
- * @file:	file to which the write happened
- * @pos:	offset where the write started
- * @count:	length of the write
- *
- * This is just a simple wrapper about our general syncing function.
- */
-int generic_write_sync(struct file *file, loff_t pos, loff_t count)
-{
-	if (!(file->f_flags & O_DSYNC) && !IS_SYNC(file->f_mapping->host))
-		return 0;
-	return vfs_fsync_range(file, pos, pos + count - 1,
-			       (file->f_flags & __O_SYNC) ? 0 : 1);
-}
-EXPORT_SYMBOL(generic_write_sync);
-
 /*
  * sys_sync_file_range() permits finely controlled syncing over a segment of
  * a file in the range offset .. (offset+nbytes-1) inclusive.  If nbytes is
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
@@ -799,7 +799,7 @@ xfs_file_aio_write(
 		XFS_STATS_ADD(xs_write_bytes, ret);
 
 		/* Handle various SYNC-type writes */
-		err = generic_write_sync(file, pos, ret);
+		err = generic_write_sync(file, iocb->ki_pos - ret, ret);
 		if (err < 0)
 			ret = err;
 	}
diff --git a/include/linux/fs.h b/include/linux/fs.h
@@ -2273,7 +2273,13 @@ extern int filemap_fdatawrite_range(struct address_space *mapping,
 extern int vfs_fsync_range(struct file *file, loff_t start, loff_t end,
 			   int datasync);
 extern int vfs_fsync(struct file *file, int datasync);
-extern int generic_write_sync(struct file *file, loff_t pos, loff_t count);
+static inline int generic_write_sync(struct file *file, loff_t pos, loff_t count)
+{
+	if (!(file->f_flags & O_DSYNC) && !IS_SYNC(file->f_mapping->host))
+		return 0;
+	return vfs_fsync_range(file, pos, pos + count - 1,
+			       (file->f_flags & __O_SYNC) ? 0 : 1);
+}
 extern void emergency_sync(void);
 extern void emergency_remount(void);
 #ifdef CONFIG_BLOCK
diff --git a/mm/filemap.c b/mm/filemap.c
@@ -2553,8 +2553,8 @@ ssize_t generic_file_aio_write(struct kiocb *iocb, const struct iovec *iov,
 	if (ret > 0) {
 		ssize_t err;
 
-		err = generic_write_sync(file, pos, ret);
-		if (err < 0 && ret > 0)
+		err = generic_write_sync(file, iocb->ki_pos - ret, ret);
+		if (err < 0)
 			ret = err;
 	}
 	return ret;

Original file line number	Diff line number	Diff line change
`@@ -2559,8 +2559,8 @@ cifs_writev(struct kiocb iocb, const struct iovec iov,`
`2559`	`2559`	`if (rc > 0) {`
`2560`	`2560`	`ssize_t err;`
`2561`	`2561`
`2562`		`- err = generic_write_sync(file, pos, rc);`
`2563`		`- if (err < 0 && rc > 0)`
	`2562`	`+ err = generic_write_sync(file, iocb->ki_pos - rc, rc);`
	`2563`	`+ if (err < 0)`
`2564`	`2564`	`rc = err;`
`2565`	`2565`	`}`
`2566`	`2566`
Original file line number	Diff line number	Diff line change
`@@ -152,7 +152,7 @@ ext4_file_dio_write(struct kiocb iocb, const struct iovec iov,`
`152`	`152`	`if (ret > 0) {`
`153`	`153`	`ssize_t err;`
`154`	`154`
`155`		`- err = generic_write_sync(file, pos, ret);`
	`155`	`+ err = generic_write_sync(file, iocb->ki_pos - ret, ret);`
`156`	`156`	`if (err < 0 && ret > 0)`
`157`	`157`	`ret = err;`
`158`	`158`	`}`
Original file line number	Diff line number	Diff line change
`@@ -2134,7 +2134,7 @@ static ssize_t ntfs_file_aio_write(struct kiocb iocb, const struct iovec iov,`
`2134`	`2134`	`ret = ntfs_file_aio_write_nolock(iocb, iov, nr_segs, &iocb->ki_pos);`
`2135`	`2135`	`mutex_unlock(&inode->i_mutex);`
`2136`	`2136`	`if (ret > 0) {`
`2137`		`- int err = generic_write_sync(file, pos, ret);`
	`2137`	`+ int err = generic_write_sync(file, iocb->ki_pos - ret, ret);`
`2138`	`2138`	`if (err < 0)`
`2139`	`2139`	`ret = err;`
`2140`	`2140`	`}`
Original file line number	Diff line number	Diff line change
`@@ -799,7 +799,7 @@ xfs_file_aio_write(`
`799`	`799`	`XFS_STATS_ADD(xs_write_bytes, ret);`
`800`	`800`
`801`	`801`	`/* Handle various SYNC-type writes */`
`802`		`- err = generic_write_sync(file, pos, ret);`
	`802`	`+ err = generic_write_sync(file, iocb->ki_pos - ret, ret);`
`803`	`803`	`if (err < 0)`
`804`	`804`	`ret = err;`
`805`	`805`	`}`