[PATCH 0/3] quota: Remove BUG_ONs from quota code

[PATCH 0/3] quota: Remove BUG_ONs from quota code

[Jan Kara]

Hello,

as has Linus notice there are several BUG_ONs in quota code that have the
potential of taking the whole machine down and they are mostly unnecessary.
This patch series changes them to WARN_ON_ONCE() and adds error handling
where appropriate. I plan to queue the series to my tree.

								Honza

[PATCH 1/3] quota: Replace BUG_ON in dqput()

[Jan Kara]

The BUG_ON in dqput() will likely take the whole machine down when it
hits. Replace it with WARN_ON_ONCE() instead and stop hiding that behind
CONFIG_QUOTA_DEBUG.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/quota/dquot.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index 31e897ad5e6a..c0d778363435 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -881,10 +881,7 @@ void dqput(struct dquot *dquot)
 	}
 
 	/* Need to release dquot? */
-#ifdef CONFIG_QUOTA_DEBUG
-	/* sanity check */
-	BUG_ON(!list_empty(&dquot->dq_free));
-#endif
+	WARN_ON_ONCE(!list_empty(&dquot->dq_free));
 	put_releasing_dquots(dquot);
 	atomic_dec(&dquot->dq_count);
 	spin_unlock(&dq_list_lock);
-- 
2.35.3

[PATCH 2/3] quota: Remove BUG_ON in dquot_load_quota_sb()

[Jan Kara]

The case when someone passes DQUOT_SUSPENDED flag to
dquot_load_quota_sb() is easy to handle. So just WARN_ON_ONCE and bail
with error if that happens instead of calling BUG_ON which is likely to
lockup the machine.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/quota/dquot.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index c0d778363435..f0bb8367641f 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -2377,7 +2377,8 @@ int dquot_load_quota_sb(struct super_block *sb, int type, int format_id,
 	lockdep_assert_held_write(&sb->s_umount);
 
 	/* Just unsuspend quotas? */
-	BUG_ON(flags & DQUOT_SUSPENDED);
+	if (WARN_ON_ONCE(flags & DQUOT_SUSPENDED))
+		return -EINVAL;
 
 	if (!fmt)
 		return -ESRCH;
-- 
2.35.3

[PATCH 3/3] quota: Remove BUG_ON from dqget()

[Jan Kara]

dqget() checks whether dquot->dq_sb is set when returning it using
BUG_ON. Firstly this doesn't work as an invalidation check for quite
some time (we release dquot with dq_sb set these days), secondly using
BUG_ON is quite harsh. Use WARN_ON_ONCE and check whether dquot is still
hashed instead.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/quota/dquot.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index f0bb8367641f..7cd83443e3eb 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -990,9 +990,8 @@ struct dquot *dqget(struct super_block *sb, struct kqid qid)
 	 * smp_mb__before_atomic() in dquot_acquire().
 	 */
 	smp_rmb();
-#ifdef CONFIG_QUOTA_DEBUG
-	BUG_ON(!dquot->dq_sb);	/* Has somebody invalidated entry under us? */
-#endif
+	/* Has somebody invalidated entry under us? */
+	WARN_ON_ONCE(hlist_unhashed(&dquot->dq_hash));
 out:
 	if (empty)
 		do_destroy_dquot(empty);
-- 
2.35.3