Re: Patch for 2.1.123 ext2 (dquota race problem)

Andrea Arcangeli (andrea@e-mind.com)
Sat, 3 Oct 1998 16:37:15 +0200 (CEST)


On Fri, 2 Oct 1998, Bill Hawes wrote:

>Dropping disk quota for a deleted inode requires writing to the quota
>file, and this may lead to attempting to lock the super block for one of
>the ext2 block allocations. But since ext2_free_inode already holds the
>superblock lock, a deadlock can result.

I don' t know when DaveM sent the report to you (I can' t see davem report
on linux-kernel). I discovered the bug _myself_ on 2.0.34 some month ago
and I developed a crude patch (note the patch has date Jul 4) against
2.0.34 that was reported working fine (also against 2.0.35) and I
explained the problem in details at that time.

Nobody answered me.

The guy able to reproduce 100% reproduce the bug is Andrea Dell'Amico btw
(the one that reported that my patch was working fine on 34 and 35).

My patch against 2.0.34 is this:

--- linux/fs/dquot.c.34 Sat Jul 4 17:27:49 1998
+++ linux/fs/dquot.c Sat Jul 4 17:32:01 1998
@@ -21,6 +21,8 @@
* removed race conditions in dqput(), dqget() and iput().
* Nick Kralevich <nickkral@cal.alumni.berkeley.edu>, 21 Jul 97
* Fixed a condition where user and group quotas could get mixed up.
+ * Removed a deadlock condition in write_dquot(),
+ * Andrea Arcangeli <arcangeli@mbox.queen.it>, 4 Jul 1998
*
* (C) Copyright 1994, 1995 Marco van Wieringen
*
@@ -36,6 +38,7 @@
#include <linux/tty.h>
#include <linux/malloc.h>
#include <linux/mount.h>
+#include <linux/locks.h>

#include <asm/segment.h>

@@ -219,6 +222,9 @@
short type = dquot->dq_type;
struct file *filp = dquot->dq_mnt->mnt_quotas[type];
unsigned short fs;
+ int write_retval;
+ unsigned char old_super_lock;
+ struct super_block *sb;

if (!(dquot->dq_flags & DQ_MOD) || (filp == (struct file *)NULL))
return;
@@ -240,8 +246,21 @@
}
fs = get_fs();
set_fs(KERNEL_DS);
- if (filp->f_op->write(filp->f_inode, filp,
- (char *)&dquot->dq_dqb, sizeof(struct dqblk)) == sizeof(struct dqblk))
+ /*
+ * Here we have the superblock locked by ext2_free_inode(), but
+ * to run without deadlock in ext2_alloc_block() we need the superblock
+ * unlocked so I try to unlock it here before the write(),
+ * _hoping_ that it will not cause fs corruption ;-).
+ */
+ sb = filp->f_inode->i_sb;
+ if ((old_super_lock = sb->s_lock))
+ unlock_super(sb);
+ write_retval = filp->f_op->write(filp->f_inode, filp,
+ (char *)&dquot->dq_dqb,
+ sizeof(struct dqblk));
+ if (old_super_lock)
+ lock_super(sb);
+ if (write_retval == sizeof(struct dqblk))
dquot->dq_flags &= ~DQ_MOD;
up(&dquot->dq_mnt->mnt_sem);
set_fs(fs);

I simply release the lock before write the dquota file. Since the
->write() will do a lock_super() before sleep everything should be fine as
far as the kernel has only one CPU running in kernel mode (2.0). Since
nobody answered me and nobody reported me problems I have not touched this
patch anymore.

Andrea[s] Arcangeli

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/