[PATCH] Forbid user to change file flags on quota files

[Jan Kara]

  Hello,

  attached patch fixes possible deadlock spotted by LIOU Payphone. If we
set inode flags for quota files, we first lock i_mutex and then start a
transaction (which is a standard ordering) but quota files are special and
their i_mutex should be acquired only when a transaction is started.
Because users have no reason to muck with inode flags when quota is on
anyway, just forbid users from changing inode flags when quota is on.
I've done the change for all the filesystems supporting quota so that
they are consistent. Andrew, would you please queue it? Thanks.

									Honza

-- 
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
---

Forbid user from changing file flags on quota files. User has no bussiness
in playing with these flags when quota is on. Furthermore there is a remote
possibility of deadlock due to a lock inversion between quota file's i_mutex and
transaction's start (i_mutex for quota file is locked only when trasaction is
started in quota operations) in ext3 and ext4.

Signed-off-by: Jan Kara <jack@xxxxxxx>
CC: LIOU Payphone <lioupayphone@xxxxxxxxx>

diff -rupX /home/jack/.kerndiffexclude linux-2.6.24-rc1/fs/ext2/ioctl.c linux-2.6.24-rc1-1-forbid_setflags/fs/ext2/ioctl.c
--- linux-2.6.24-rc1/fs/ext2/ioctl.c	2007-10-24 20:43:56.000000000 +0200
+++ linux-2.6.24-rc1-1-forbid_setflags/fs/ext2/ioctl.c	2007-11-01 16:01:26.000000000 +0100
@@ -47,6 +47,11 @@ int ext2_ioctl (struct inode * inode, st
 			flags &= ~EXT2_DIRSYNC_FL;
 
 		mutex_lock(&inode->i_mutex);
+		/* Is it quota file? Don't allow user mess with it */
+		if (IS_NOQUOTA(inode)) {
+			mutex_unlock(&inode->i_mutex);
+			return -EPERM;
+		}
 		oldflags = ei->i_flags;
 
 		/*
diff -rupX /home/jack/.kerndiffexclude linux-2.6.24-rc1/fs/ext3/ioctl.c linux-2.6.24-rc1-1-forbid_setflags/fs/ext3/ioctl.c
--- linux-2.6.24-rc1/fs/ext3/ioctl.c	2007-10-11 12:01:23.000000000 +0200
+++ linux-2.6.24-rc1-1-forbid_setflags/fs/ext3/ioctl.c	2007-11-01 16:01:58.000000000 +0100
@@ -51,6 +51,11 @@ int ext3_ioctl (struct inode * inode, st
 			flags &= ~EXT3_DIRSYNC_FL;
 
 		mutex_lock(&inode->i_mutex);
+		/* Is it quota file? Don't allow user mess with it */
+		if (IS_NOQUOTA(inode)) {
+			mutex_unlock(&inode->i_mutex);
+			return -EPERM;
+		}
 		oldflags = ei->i_flags;
 
 		/* The JOURNAL_DATA flag is modifiable only by root */
diff -rupX /home/jack/.kerndiffexclude linux-2.6.24-rc1/fs/ext4/ioctl.c linux-2.6.24-rc1-1-forbid_setflags/fs/ext4/ioctl.c
--- linux-2.6.24-rc1/fs/ext4/ioctl.c	2007-10-11 12:01:23.000000000 +0200
+++ linux-2.6.24-rc1-1-forbid_setflags/fs/ext4/ioctl.c	2007-11-01 16:03:02.000000000 +0100
@@ -51,6 +51,11 @@ int ext4_ioctl (struct inode * inode, st
 			flags &= ~EXT4_DIRSYNC_FL;
 
 		mutex_lock(&inode->i_mutex);
+		/* Is it quota file? Don't allow user mess with it */
+		if (IS_NOQUOTA(inode)) {
+			mutex_unlock(&inode->i_mutex);
+			return -EPERM;
+		}
 		oldflags = ei->i_flags;
 
 		/* The JOURNAL_DATA flag is modifiable only by root */
diff -rupX /home/jack/.kerndiffexclude linux-2.6.24-rc1/fs/jfs/ioctl.c linux-2.6.24-rc1-1-forbid_setflags/fs/jfs/ioctl.c
--- linux-2.6.24-rc1/fs/jfs/ioctl.c	2007-10-11 12:01:23.000000000 +0200
+++ linux-2.6.24-rc1-1-forbid_setflags/fs/jfs/ioctl.c	2007-11-01 16:13:07.000000000 +0100
@@ -79,6 +79,9 @@ int jfs_ioctl(struct inode * inode, stru
 		if (!S_ISDIR(inode->i_mode))
 			flags &= ~JFS_DIRSYNC_FL;
 
+		/* Is it quota file? Don't allow user mess with it */
+		if (IS_NOQUOTA(inode))
+			return -EPERM;
 		jfs_get_inode_flags(jfs_inode);
 		oldflags = jfs_inode->mode2;
 
diff -rupX /home/jack/.kerndiffexclude linux-2.6.24-rc1/fs/reiserfs/ioctl.c linux-2.6.24-rc1-1-forbid_setflags/fs/reiserfs/ioctl.c
--- linux-2.6.24-rc1/fs/reiserfs/ioctl.c	2007-10-24 20:43:58.000000000 +0200
+++ linux-2.6.24-rc1-1-forbid_setflags/fs/reiserfs/ioctl.c	2007-11-01 16:13:27.000000000 +0100
@@ -57,6 +57,9 @@ int reiserfs_ioctl(struct inode *inode, 
 			if (get_user(flags, (int __user *)arg))
 				return -EFAULT;
 
+			/* Is it quota file? Don't allow user mess with it. */
+			if (IS_NOQUOTA(inode))
+				return -EPERM;
 			if (((flags ^ REISERFS_I(inode)->
 			      i_attrs) & (REISERFS_IMMUTABLE_FL |
 					  REISERFS_APPEND_FL))
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/