[PATCH 0/2] ext4: Quota enabling error handling fixes

[PATCH 0/2] ext4: Quota enabling error handling fixes

[Jan Kara]

Hello,

when enabling of quotas fails in the middle, we could be leaving quota
subsystem in unexpected state or free inodes with locking classes set.
Make sure to do proper cleanup in case quota setup fails.

								Honza

[PATCH 1/2] ext4: Make sure quota gets properly shutdown on error

[Jan Kara]

When we hit an error when enabling quotas and setting inode flags, we do
not properly shutdown quota subsystem despite returning error from
Q_QUOTAON quotactl. This can lead to some odd situations like kernel
using quota file while it is still writeable for userspace. Make sure we
properly cleanup the quota subsystem in case of error.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/super.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 88d5d274a868..fbe9cae63786 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -6267,10 +6267,7 @@ static int ext4_quota_on(struct super_block *sb, int type, int format_id,
 
 	lockdep_set_quota_inode(path->dentry->d_inode, I_DATA_SEM_QUOTA);
 	err = dquot_quota_on(sb, type, format_id, path);
-	if (err) {
-		lockdep_set_quota_inode(path->dentry->d_inode,
-					     I_DATA_SEM_NORMAL);
-	} else {
+	if (!err) {
 		struct inode *inode = d_inode(path->dentry);
 		handle_t *handle;
 
@@ -6290,7 +6287,12 @@ static int ext4_quota_on(struct super_block *sb, int type, int format_id,
 		ext4_journal_stop(handle);
 	unlock_inode:
 		inode_unlock(inode);
+		if (err)
+			dquot_quota_off(sb, type);
 	}
+	if (err)
+		lockdep_set_quota_inode(path->dentry->d_inode,
+					     I_DATA_SEM_NORMAL);
 	return err;
 }
 
-- 
2.26.2

[PATCH 2/2] ext4: Make sure to reset inode lockdep class when quota enabling fails

[Jan Kara]

When we succeed in enabling some quota type but fail to enable another
one with quota feature, we correctly disable all enabled quota types.
However we forget to reset i_data_sem lockdep class. When the inode gets
freed and reused, it will inherit this lockdep class (i_data_sem is
initialized only when a slab is created) and thus eventually lockdep
barfs about possible deadlocks.

Reported-and-tested-by: syzbot+3b6f9218b1301ddda3e2@syzkaller.appspotmail.com
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/super.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index fbe9cae63786..70b5fcbd351a 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -6355,8 +6355,19 @@ int ext4_enable_quotas(struct super_block *sb)
 					"Failed to enable quota tracking "
 					"(type=%d, err=%d). Please run "
 					"e2fsck to fix.", type, err);
-				for (type--; type >= 0; type--)
+				for (type--; type >= 0; type--) {
+					struct inode *inode;
+
+					inode = sb_dqopt(sb)->files[type];
+					if (inode)
+						inode = igrab(inode);
 					dquot_quota_off(sb, type);
+					if (inode) {
+						lockdep_set_quota_inode(inode,
+							I_DATA_SEM_NORMAL);
+						iput(inode);
+					}
+				}
 
 				return err;
 			}
-- 
2.26.2

Re: [PATCH 0/2] ext4: Quota enabling error handling fixes

[Theodore Ts'o]

On Thu, 7 Oct 2021 17:53:34 +0200, Jan Kara wrote:
> when enabling of quotas fails in the middle, we could be leaving quota
> subsystem in unexpected state or free inodes with locking classes set.
> Make sure to do proper cleanup in case quota setup fails.
> 
> Honza
> 

Applied, thanks!

[1/2] ext4: Make sure quota gets properly shutdown on error
      commit: 298503cc5015e0512294788127a853a730d47fb1
[2/2] ext4: Make sure to reset inode lockdep class when quota enabling fails
      commit: c5725ba32f98d128d5d83cab6d0b24ceb4ecf731

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>