[PATCH] xfs: take XFS_IOLOCK_EXCL if suid removal is required

* [PATCH] xfs: take XFS_IOLOCK_EXCL if suid removal is required
@ 2015-03-06 16:54 Eric Sandeen
  2015-03-06 21:09 ` Dave Chinner
  0 siblings, 1 reply; 5+ messages in thread
From: Eric Sandeen @ 2015-03-06 16:54 UTC (permalink / raw)
  To: xfs-oss

This sequence:

# rm -f sparsefile
# truncate --size=1m sparsefile
# chmod ugo+rws sparsefile
# ls -l sparsefile
-rwSrwSrw-. 1 root root 1048576 Mar  6 10:29 sparsefile
# su tester

$ xfs_io -d -c "pwrite 0 4096" sparsefile
wrote 4096/4096 bytes at offset 0
4 KiB, 1 ops; 0.0000 sec (21.505 KiB/sec and 5.3763 ops/sec)
$ exit

will lead to a WARN_ON() in notify change, because i_mutex is
not held, and we get to notify_change via suid removal with
only XFS_IOLOCK_SHARED held, i.e. no i_mutex.

Upgrade the lock to XFS_IOLOCK_EXCL in this case.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index a2e1cb8..e64d5b1 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -541,7 +541,7 @@ xfs_zero_eof(
  *
  * Called with the iolocked held either shared and exclusive according to
  * @iolock, and returns with it held.  Might upgrade the iolock to exclusive
- * if called for a direct write beyond i_size.
+ * if called for a direct write beyond i_size or if suid removal is required.
  */
 STATIC ssize_t
 xfs_file_aio_write_checks(
@@ -551,6 +551,7 @@ xfs_file_aio_write_checks(
 	int			*iolock)
 {
 	struct inode		*inode = file->f_mapping->host;
+	struct dentry		*dentry = file->f_path.dentry;
 	struct xfs_inode	*ip = XFS_I(inode);
 	int			error = 0;
 
@@ -569,9 +570,12 @@ restart:
 	 * write.  If zeroing is needed and we are currently holding the
 	 * iolock shared, we need to update it to exclusive which implies
 	 * having to redo all checks before.
+	 *
+	 * We also must be locked exclusively if this write will require
+	 * suid removal; notify_change expects i_mutex to be locked.
 	 */
-	if (*pos > i_size_read(inode)) {
-		bool	zero = false;
+	if (*pos > i_size_read(inode) || should_remove_suid(dentry)) {
+		bool	z = false;
 
 		if (*iolock == XFS_IOLOCK_SHARED) {
 			xfs_rw_iunlock(ip, *iolock);
@@ -579,9 +583,12 @@ restart:
 			xfs_rw_ilock(ip, *iolock);
 			goto restart;
 		}
-		error = xfs_zero_eof(ip, *pos, i_size_read(inode), &zero);
-		if (error)
-			return error;
+
+		if (*pos > i_size_read(inode) ) {
+			error = xfs_zero_eof(ip, *pos, i_size_read(inode), &z);
+			if (error)
+				return error;
+		}
 	}
 
 	/*


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 5+ messages in thread