Messages in this thread Patch in this message | | | Date | Sat, 3 Oct 1998 16:37:15 +0200 (CEST) | From | Andrea Arcangeli <> | Subject | Re: Patch for 2.1.123 ext2 (dquota race problem) |
| |
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/
| |