BUG: reiserfs+acl+quota deadlock

BUG: reiserfs+acl+quota deadlock

[Tarmo Tänav]

Hi,

I've already reported a similiar bug to the one I found now
and that was fixed by:
"[PATCH] reiserfs: fix deadlock in inode creation failure path w/
default ACL"

This bug is similiar in effect but has some differences in how
to trigger it. The end effect will be just like with the other
bug that the affected directory will be unaccessible to any user
or process.

So here's the way to reproduce it, as minimal as I could get it:

You need reiserfs, quota and acl support in kernel.
you also need quota tools (edquota, quotaon, quotacheck), I used
linuxquota 3.12.

# cd /mnt
# dd if=/dev/zero of=test bs=1M count=50
50+0 records in
50+0 records out
# mkreiserfs -f test >/dev/null
mkreiserfs 3.6.19 (2003 www.namesys.com)

test is not a block special device
Continue (y/n):y
# mkdir mpoint
# mount test mpoint -o loop,acl,usrquota
# mkdir mpoint/user1
# useradd -d /mnt/mpoint/user1 user1     # may also use existing user
# chown user1 mpoint/user1
# quotacheck -v mpoint                   # initializes quota file
# edquota user1
---- set soft block limit to 1000, hard limit to 4000 ----
# edquota -t
---- set the grace periods to something small: 1minutes ---
# quotaon mpoint
# ## at this point "repquota -a" should show the quota for user1
# su user1
# cd
# ## now we are in user1 home dir as user1
# cat /dev/zero > file1
loop2: warning, user block quota exceeded.
loop2: write failed, user block limit reached.
cat: write error: No space left on device
--- now we wait till the grace period expires (repquota -a) ----
# cat "" > otherfile
loop2: write failed, user block quota exceeded too long.
---- and it will hang forever ----
# ## /mnt/mpoint can still be accessed, but /mnt/mpoint/user1 can't


I tested this on an -mm patchset kernel (2.6.13-rc5-mm1), but I
discovered the bug in my server which runs plain 2.6.12 with the
patch from Jeff Mahoney for the first reiserfs+acl bug.

The main difference between the two bugs is that the first one requires
the existance of a default acl, this one does not, but it does require
acl to be enabled.


PS. please CC, I'm not subscribed to the list

-- 
Tarmo Tänav <tarmo@itech.ee>

Re: BUG: reiserfs+acl+quota deadlock

[Jan Kara]

[-- Attachment #1: Type: text/plain, Size: 2421 bytes --]

  Hello,

> I've already reported a similiar bug to the one I found now
> and that was fixed by:
> "[PATCH] reiserfs: fix deadlock in inode creation failure path w/
> default ACL"
> 
> This bug is similiar in effect but has some differences in how
> to trigger it. The end effect will be just like with the other
> bug that the affected directory will be unaccessible to any user
> or process.
> 
> So here's the way to reproduce it, as minimal as I could get it:
> 
> You need reiserfs, quota and acl support in kernel.
> you also need quota tools (edquota, quotaon, quotacheck), I used
> linuxquota 3.12.
> 
> # cd /mnt
> # dd if=/dev/zero of=test bs=1M count=50
> 50+0 records in
> 50+0 records out
> # mkreiserfs -f test >/dev/null
> mkreiserfs 3.6.19 (2003 www.namesys.com)
> 
> test is not a block special device
> Continue (y/n):y
> # mkdir mpoint
> # mount test mpoint -o loop,acl,usrquota
> # mkdir mpoint/user1
> # useradd -d /mnt/mpoint/user1 user1     # may also use existing user
> # chown user1 mpoint/user1
> # quotacheck -v mpoint                   # initializes quota file
> # edquota user1
> ---- set soft block limit to 1000, hard limit to 4000 ----
> # edquota -t
> ---- set the grace periods to something small: 1minutes ---
> # quotaon mpoint
> # ## at this point "repquota -a" should show the quota for user1
> # su user1
> # cd
> # ## now we are in user1 home dir as user1
> # cat /dev/zero > file1
> loop2: warning, user block quota exceeded.
> loop2: write failed, user block limit reached.
> cat: write error: No space left on device
> --- now we wait till the grace period expires (repquota -a) ----
> # cat "" > otherfile
> loop2: write failed, user block quota exceeded too long.
> ---- and it will hang forever ----
> # ## /mnt/mpoint can still be accessed, but /mnt/mpoint/user1 can't
> 
> 
> I tested this on an -mm patchset kernel (2.6.13-rc5-mm1), but I
> discovered the bug in my server which runs plain 2.6.12 with the
> patch from Jeff Mahoney for the first reiserfs+acl bug.
> 
> The main difference between the two bugs is that the first one requires
> the existance of a default acl, this one does not, but it does require
> acl to be enabled.
  This seems to be the same problem as bug #4771 that I've just fix. Can
you try attached patch please?
  Andrew, can you include the patch into -mm if ReiserFS guys won't object?

								Honza
-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs

[-- Attachment #2: reiser-2.6.13-rc6-create_fix.diff --]
[-- Type: text/plain, Size: 821 bytes --]

Initialize key object ID in inode so that we don't try to remove the inode
when we fail on some checks even before we manage to allocate something.

Signed-off-by: Jan Kara <jack@suse.cz>

diff -rupX /home/jack/.kerndiffexclude linux-2.6.13-rc6/fs/reiserfs/namei.c linux-2.6.13-rc6-reiser_create_fix/fs/reiserfs/namei.c
--- linux-2.6.13-rc6/fs/reiserfs/namei.c	2005-08-12 10:39:25.000000000 +0200
+++ linux-2.6.13-rc6-reiser_create_fix/fs/reiserfs/namei.c	2005-08-12 10:39:07.000000000 +0200
@@ -593,6 +593,9 @@ static int new_inode_init(struct inode *
 	 */
 	inode->i_uid = current->fsuid;
 	inode->i_mode = mode;
+	/* Make inode invalid - just in case we are going to drop it before
+	 * the initialization happens */
+	INODE_PKEY(inode)->k_objectid = 0;
 
 	if (dir->i_mode & S_ISGID) {
 		inode->i_gid = dir->i_gid;

Re: BUG: reiserfs+acl+quota deadlock

[Tarmo Tänav]

Tried the attached patch but it changed nothing, I trying to create
a new file as a user whose quota grace time has ran out will still
cause everything accessing the users homedir (the one with the quota)
to hang in D state.

Also note that the bug I reported only exists when acl is also
enabled (does not have to be used). And although my kernel is not
built with debug (or reiserfs debug) support, I don't get any
oopses or reiserfs errors.. it just hangs.


On K, 2005-08-10 at 15:00 +0200, Jan Kara wrote:
>   Hello,
> 
> > I've already reported a similiar bug to the one I found now
> > and that was fixed by:
> > "[PATCH] reiserfs: fix deadlock in inode creation failure path w/
> > default ACL"
> > 
> > This bug is similiar in effect but has some differences in how
> > to trigger it. The end effect will be just like with the other
> > bug that the affected directory will be unaccessible to any user
> > or process.
> > 
> > So here's the way to reproduce it, as minimal as I could get it:
> > 
> > You need reiserfs, quota and acl support in kernel.
> > you also need quota tools (edquota, quotaon, quotacheck), I used
> > linuxquota 3.12.
> > 
> > # cd /mnt
> > # dd if=/dev/zero of=test bs=1M count=50
> > 50+0 records in
> > 50+0 records out
> > # mkreiserfs -f test >/dev/null
> > mkreiserfs 3.6.19 (2003 www.namesys.com)
> > 
> > test is not a block special device
> > Continue (y/n):y
> > # mkdir mpoint
> > # mount test mpoint -o loop,acl,usrquota
> > # mkdir mpoint/user1
> > # useradd -d /mnt/mpoint/user1 user1     # may also use existing user
> > # chown user1 mpoint/user1
> > # quotacheck -v mpoint                   # initializes quota file
> > # edquota user1
> > ---- set soft block limit to 1000, hard limit to 4000 ----
> > # edquota -t
> > ---- set the grace periods to something small: 1minutes ---
> > # quotaon mpoint
> > # ## at this point "repquota -a" should show the quota for user1
> > # su user1
> > # cd
> > # ## now we are in user1 home dir as user1
> > # cat /dev/zero > file1
> > loop2: warning, user block quota exceeded.
> > loop2: write failed, user block limit reached.
> > cat: write error: No space left on device
> > --- now we wait till the grace period expires (repquota -a) ----
> > # cat "" > otherfile
> > loop2: write failed, user block quota exceeded too long.
> > ---- and it will hang forever ----
> > # ## /mnt/mpoint can still be accessed, but /mnt/mpoint/user1 can't
> > 
> > 
> > I tested this on an -mm patchset kernel (2.6.13-rc5-mm1), but I
> > discovered the bug in my server which runs plain 2.6.12 with the
> > patch from Jeff Mahoney for the first reiserfs+acl bug.
> > 
> > The main difference between the two bugs is that the first one requires
> > the existance of a default acl, this one does not, but it does require
> > acl to be enabled.
>   This seems to be the same problem as bug #4771 that I've just fix. Can
> you try attached patch please?
>   Andrew, can you include the patch into -mm if ReiserFS guys won't object?

Re: BUG: reiserfs+acl+quota deadlock

[Jan Kara]

> Tried the attached patch but it changed nothing, I trying to create
> a new file as a user whose quota grace time has ran out will still
> cause everything accessing the users homedir (the one with the quota)
> to hang in D state.
> 
> Also note that the bug I reported only exists when acl is also
> enabled (does not have to be used). And although my kernel is not
> built with debug (or reiserfs debug) support, I don't get any
> oopses or reiserfs errors.. it just hangs.
  Oops, sorry. I forgot to mount the fs with ACL mount option and so I
was not able to reproduce the hang. My fault, your bug is a different
problem. Now it hangs also for me so I can debug it :)

								Honza

> On K, 2005-08-10 at 15:00 +0200, Jan Kara wrote:
> >   Hello,
> > 
> > > I've already reported a similiar bug to the one I found now
> > > and that was fixed by:
> > > "[PATCH] reiserfs: fix deadlock in inode creation failure path w/
> > > default ACL"
> > > 
> > > This bug is similiar in effect but has some differences in how
> > > to trigger it. The end effect will be just like with the other
> > > bug that the affected directory will be unaccessible to any user
> > > or process.
> > > 
> > > So here's the way to reproduce it, as minimal as I could get it:
> > > 
> > > You need reiserfs, quota and acl support in kernel.
> > > you also need quota tools (edquota, quotaon, quotacheck), I used
> > > linuxquota 3.12.
> > > 
> > > # cd /mnt
> > > # dd if=/dev/zero of=test bs=1M count=50
> > > 50+0 records in
> > > 50+0 records out
> > > # mkreiserfs -f test >/dev/null
> > > mkreiserfs 3.6.19 (2003 www.namesys.com)
> > > 
> > > test is not a block special device
> > > Continue (y/n):y
> > > # mkdir mpoint
> > > # mount test mpoint -o loop,acl,usrquota
> > > # mkdir mpoint/user1
> > > # useradd -d /mnt/mpoint/user1 user1     # may also use existing user
> > > # chown user1 mpoint/user1
> > > # quotacheck -v mpoint                   # initializes quota file
> > > # edquota user1
> > > ---- set soft block limit to 1000, hard limit to 4000 ----
> > > # edquota -t
> > > ---- set the grace periods to something small: 1minutes ---
> > > # quotaon mpoint
> > > # ## at this point "repquota -a" should show the quota for user1
> > > # su user1
> > > # cd
> > > # ## now we are in user1 home dir as user1
> > > # cat /dev/zero > file1
> > > loop2: warning, user block quota exceeded.
> > > loop2: write failed, user block limit reached.
> > > cat: write error: No space left on device
> > > --- now we wait till the grace period expires (repquota -a) ----
> > > # cat "" > otherfile
> > > loop2: write failed, user block quota exceeded too long.
> > > ---- and it will hang forever ----
> > > # ## /mnt/mpoint can still be accessed, but /mnt/mpoint/user1 can't
> > > 
> > > 
> > > I tested this on an -mm patchset kernel (2.6.13-rc5-mm1), but I
> > > discovered the bug in my server which runs plain 2.6.12 with the
> > > patch from Jeff Mahoney for the first reiserfs+acl bug.
> > > 
> > > The main difference between the two bugs is that the first one requires
> > > the existance of a default acl, this one does not, but it does require
> > > acl to be enabled.
> >   This seems to be the same problem as bug #4771 that I've just fix. Can
> > you try attached patch please?
> >   Andrew, can you include the patch into -mm if ReiserFS guys won't object?
> 
-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs

Re: BUG: reiserfs+acl+quota deadlock

[Vladimir V. Saveliev]

[-- Attachment #1: Type: text/plain, Size: 885 bytes --]

Hello

Jan Kara wrote:
>>Tried the attached patch but it changed nothing, I trying to create
>>a new file as a user whose quota grace time has ran out will still
>>cause everything accessing the users homedir (the one with the quota)
>>to hang in D state.
>>
>>Also note that the bug I reported only exists when acl is also
>>enabled (does not have to be used). And although my kernel is not
>>built with debug (or reiserfs debug) support, I don't get any
>>oopses or reiserfs errors.. it just hangs.
> 

It looks like the problem is that reiserfs_new_inode can be called either having xattrs locked or not.
It does unlocking/locking xattrs on error handling path, but has no idea about whether
xattrs are locked of not.
The attached patch seems to fix the problem.
I am not sure whether it is correct way to fix this problem, though.

Tarmo, please check if this patch works for you.

[-- Attachment #2: reiserfs-fix-xattr-deadlock.patch --]
[-- Type: text/plain, Size: 3597 bytes --]

 fs/reiserfs/inode.c         |   13 +++++++++----
 fs/reiserfs/namei.c         |    8 ++++----
 include/linux/reiserfs_fs.h |    2 +-
 3 files changed, 14 insertions(+), 9 deletions(-)

diff -puN fs/reiserfs/inode.c~reiserfs-debug-1 fs/reiserfs/inode.c
--- linux-2.6.13-rc4-mm1/fs/reiserfs/inode.c~reiserfs-debug-1	2005-08-12 15:25:13.548577536 +0400
+++ linux-2.6.13-rc4-mm1-vs/fs/reiserfs/inode.c	2005-08-12 18:49:08.384891765 +0400
@@ -1776,7 +1776,7 @@ int reiserfs_new_inode(struct reiserfs_t
 		       /* 0 for regular, EMTRY_DIR_SIZE for dirs, 
 		          strlen (symname) for symlinks) */
 		       loff_t i_size, struct dentry *dentry,
-		       struct inode *inode)
+		       struct inode *inode, int locked)
 {
 	struct super_block *sb;
 	INITIALIZE_PATH(path_to_key);
@@ -1966,6 +1966,7 @@ int reiserfs_new_inode(struct reiserfs_t
  * are place holders for what the quota code actually needs.
  */
       out_bad_inode:
+
 	/* Invalidate the object, nothing was inserted yet */
 	INODE_PKEY(inode)->k_objectid = 0;
 
@@ -1988,11 +1989,15 @@ int reiserfs_new_inode(struct reiserfs_t
 	 * code really needs to be reworked, but this will take care of it
 	 * for now. -jeffm */
 	if (REISERFS_I(dir)->i_acl_default) {
-		reiserfs_write_unlock_xattrs(dir->i_sb);
+		if (locked)
+			reiserfs_write_unlock_xattrs(dir->i_sb);
 		iput(inode);
-		reiserfs_write_lock_xattrs(dir->i_sb);
-	} else
+		if (locked)
+			reiserfs_write_lock_xattrs(dir->i_sb);
+	} else {
 		iput(inode);
+	}
+
 	return err;
 }
 
diff -puN fs/reiserfs/namei.c~reiserfs-debug-1 fs/reiserfs/namei.c
--- linux-2.6.13-rc4-mm1/fs/reiserfs/namei.c~reiserfs-debug-1	2005-08-12 18:48:29.985413281 +0400
+++ linux-2.6.13-rc4-mm1-vs/fs/reiserfs/namei.c	2005-08-12 18:48:30.061420166 +0400
@@ -638,7 +638,7 @@ static int reiserfs_create(struct inode 
 
 	retval =
 	    reiserfs_new_inode(&th, dir, mode, NULL, 0 /*i_size */ , dentry,
-			       inode);
+			       inode, locked);
 	if (retval)
 		goto out_failed;
 
@@ -713,7 +713,7 @@ static int reiserfs_mknod(struct inode *
 
 	retval =
 	    reiserfs_new_inode(&th, dir, mode, NULL, 0 /*i_size */ , dentry,
-			       inode);
+			       inode, locked);
 	if (retval) {
 		goto out_failed;
 	}
@@ -798,7 +798,7 @@ static int reiserfs_mkdir(struct inode *
 	    retval = reiserfs_new_inode(&th, dir, mode, NULL /*symlink */ ,
 					old_format_only(dir->i_sb) ?
 					EMPTY_DIR_SIZE_V1 : EMPTY_DIR_SIZE,
-					dentry, inode);
+					dentry, inode, locked);
 	if (retval) {
 		dir->i_nlink--;
 		goto out_failed;
@@ -1086,7 +1086,7 @@ static int reiserfs_symlink(struct inode
 
 	retval =
 	    reiserfs_new_inode(&th, parent_dir, mode, name, strlen(symname),
-			       dentry, inode);
+			       dentry, inode, 0);
 	reiserfs_kfree(name, item_len, parent_dir->i_sb);
 	if (retval) {		/* reiserfs_new_inode iputs for us */
 		goto out_failed;
diff -puN include/linux/reiserfs_fs.h~reiserfs-debug-1 include/linux/reiserfs_fs.h
--- linux-2.6.13-rc4-mm1/include/linux/reiserfs_fs.h~reiserfs-debug-1	2005-08-12 18:48:30.041418354 +0400
+++ linux-2.6.13-rc4-mm1-vs/include/linux/reiserfs_fs.h	2005-08-12 18:48:30.061420166 +0400
@@ -1890,7 +1890,7 @@ struct inode *reiserfs_iget(struct super
 int reiserfs_new_inode(struct reiserfs_transaction_handle *th,
 		       struct inode *dir, int mode,
 		       const char *symname, loff_t i_size,
-		       struct dentry *dentry, struct inode *inode);
+		       struct dentry *dentry, struct inode *inode, int locked);
 
 void reiserfs_update_sd_size(struct reiserfs_transaction_handle *th,
 			     struct inode *inode, loff_t size);

_

Re: BUG: reiserfs+acl+quota deadlock

[Jeff Mahoney]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1




Vladimir V. Saveliev wrote:
> Hello
> 
> Jan Kara wrote:
>>> Tried the attached patch but it changed nothing, I trying to create
>>> a new file as a user whose quota grace time has ran out will still
>>> cause everything accessing the users homedir (the one with the quota)
>>> to hang in D state.
>>>
>>> Also note that the bug I reported only exists when acl is also
>>> enabled (does not have to be used). And although my kernel is not
>>> built with debug (or reiserfs debug) support, I don't get any
>>> oopses or reiserfs errors.. it just hangs.
>>
> 
> It looks like the problem is that reiserfs_new_inode can be called
> either having xattrs locked or not.
> It does unlocking/locking xattrs on error handling path, but has no idea
> about whether
> xattrs are locked of not.
> The attached patch seems to fix the problem.
> I am not sure whether it is correct way to fix this problem, though.

Does this patch actually fix it? It shouldn't.

The logic is like this: If a default ACL is associated with the parent
when the inode is created, xattrs will be locked so that the ACL can be
inherited. Since reiserfs_new_inode is called from the VFS layer with
inode->i_sem downed, {set,remove}xattr is locked out. The default ACL
can't be removed/added/changed while reiserfs_new_inode is running.
Therefore, if there is a default ACL, xattrs must be locked.

- -Jeff

- --
Jeff Mahoney
SuSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (GNU/Linux)

iD8DBQFC/LwALPWxlyuTD7IRAl1hAJ9dVKCWPYdMO85+EKjL+2kq9dy3ngCfdS9w
56060gxdR2z0d6UFP79yQ1A=
=S8+3
-----END PGP SIGNATURE-----

Re: BUG: reiserfs+acl+quota deadlock

[Tarmo Tänav]

Vladimir V. Saveliev wrote:
>  
> > It looks like the problem is that reiserfs_new_inode can be called
> > either having xattrs locked or not.
> > It does unlocking/locking xattrs on error handling path, but has no idea
> > about whether
> > xattrs are locked of not.
> > The attached patch seems to fix the problem.
> > I am not sure whether it is correct way to fix this problem, though.

I tested the patch, it did not fix the problem. Is there any way that I
could help diagnose the problem? (so far I have not tried reiserfs or
kernel debug options, could those help?)


On R, 2005-08-12 at 11:10 -0400, Jeff Mahoney wrote:
> 
> Does this patch actually fix it? It shouldn't.
> 
> The logic is like this: If a default ACL is associated with the parent
> when the inode is created, xattrs will be locked so that the ACL can be
> inherited. Since reiserfs_new_inode is called from the VFS layer with
> inode->i_sem downed, {set,remove}xattr is locked out. The default ACL
> can't be removed/added/changed while reiserfs_new_inode is running.
> Therefore, if there is a default ACL, xattrs must be locked.

I don't know if you read my report on this bug, but note that it had
no requirement for any ACL's to be used (even default ACL's), there was
only need for acl support to be enabled when mounting the partition.



-- 
Tarmo Tänav <tarmo@itech.ee>

Re: BUG: reiserfs+acl+quota deadlock

[Tarmo Tänav]

On R, 2005-08-12 at 18:55 +0400, Vladimir V. Saveliev wrote:
> 
> It looks like the problem is that reiserfs_new_inode can be called either having xattrs locked or not.
> It does unlocking/locking xattrs on error handling path, but has no idea about whether
> xattrs are locked of not.
> The attached patch seems to fix the problem.
> I am not sure whether it is correct way to fix this problem, though.
> 
> Tarmo, please check if this patch works for you.


Sorry, I made a mistake when I first tested the patch, got some
kernel images mixed up, it does fix the problem, thank you.



-- 
Tarmo Tänav <tarmo@itech.ee>

Re: BUG: reiserfs+acl+quota deadlock

[Jan Kara]

[-- Attachment #1: Type: text/plain, Size: 1150 bytes --]

  Hello,

> Jan Kara wrote:
> >>Tried the attached patch but it changed nothing, I trying to create
> >>a new file as a user whose quota grace time has ran out will still
> >>cause everything accessing the users homedir (the one with the quota)
> >>to hang in D state.
> >>
> >>Also note that the bug I reported only exists when acl is also
> >>enabled (does not have to be used). And although my kernel is not
> >>built with debug (or reiserfs debug) support, I don't get any
> >>oopses or reiserfs errors.. it just hangs.
> >
> 
> It looks like the problem is that reiserfs_new_inode can be called either 
> having xattrs locked or not.
> It does unlocking/locking xattrs on error handling path, but has no idea 
> about whether
> xattrs are locked of not.
> The attached patch seems to fix the problem.
> I am not sure whether it is correct way to fix this problem, though.
  I've already fixed this problem and Andrew accepted the patch into
-mm. I took a bit different approach but yours might be better in a long
run (mine is just a one liner). The patch is attached if you're
interested.

								Honza
--
Jan Kara <jack@suse.cz>
SuSE CR Labs

[-- Attachment #2: reiser-2.6.13-rc6-2-xattr_fix.diff --]
[-- Type: text/plain, Size: 956 bytes --]

When i_acl_default is set to some error we do not hold the lock (hence we are
not allowed to drop it and reacquire later).

Signed-off-by: Jan Kara <jack@suse.cz>

diff -rupX /home/jack/.kerndiffexclude linux-2.6.13-rc6-1-reiser_create_fix/fs/reiserfs/inode.c linux-2.6.13-rc6-2-reiser_xattr_fix/fs/reiserfs/inode.c
--- linux-2.6.13-rc6-1-reiser_create_fix/fs/reiserfs/inode.c	2005-08-14 17:10:21.000000000 +0200
+++ linux-2.6.13-rc6-2-reiser_xattr_fix/fs/reiserfs/inode.c	2005-08-14 17:11:35.000000000 +0200
@@ -1985,7 +1985,7 @@ int reiserfs_new_inode(struct reiserfs_t
 	 * iput doesn't deadlock in reiserfs_delete_xattrs. The locking
 	 * code really needs to be reworked, but this will take care of it
 	 * for now. -jeffm */
-	if (REISERFS_I(dir)->i_acl_default) {
+	if (REISERFS_I(dir)->i_acl_default && !IS_ERR(REISERFS_I(dir)->i_acl_default)) {
 		reiserfs_write_unlock_xattrs(dir->i_sb);
 		iput(inode);
 		reiserfs_write_lock_xattrs(dir->i_sb);

Re: BUG: reiserfs+acl+quota deadlock

[Jan Kara]

[-- Attachment #1: Type: text/plain, Size: 3322 bytes --]

> Tried the attached patch but it changed nothing, I trying to create
> a new file as a user whose quota grace time has ran out will still
> cause everything accessing the users homedir (the one with the quota)
> to hang in D state.
> 
> Also note that the bug I reported only exists when acl is also
> enabled (does not have to be used). And although my kernel is not
> built with debug (or reiserfs debug) support, I don't get any
> oopses or reiserfs errors.. it just hangs.
  OK, I've debugged the hang (I think the bug was actually introduced by
Jeff's fix). Attached patch should fix it.

								Honza

> On K, 2005-08-10 at 15:00 +0200, Jan Kara wrote:
> >   Hello,
> > 
> > > I've already reported a similiar bug to the one I found now
> > > and that was fixed by:
> > > "[PATCH] reiserfs: fix deadlock in inode creation failure path w/
> > > default ACL"
> > > 
> > > This bug is similiar in effect but has some differences in how
> > > to trigger it. The end effect will be just like with the other
> > > bug that the affected directory will be unaccessible to any user
> > > or process.
> > > 
> > > So here's the way to reproduce it, as minimal as I could get it:
> > > 
> > > You need reiserfs, quota and acl support in kernel.
> > > you also need quota tools (edquota, quotaon, quotacheck), I used
> > > linuxquota 3.12.
> > > 
> > > # cd /mnt
> > > # dd if=/dev/zero of=test bs=1M count=50
> > > 50+0 records in
> > > 50+0 records out
> > > # mkreiserfs -f test >/dev/null
> > > mkreiserfs 3.6.19 (2003 www.namesys.com)
> > > 
> > > test is not a block special device
> > > Continue (y/n):y
> > > # mkdir mpoint
> > > # mount test mpoint -o loop,acl,usrquota
> > > # mkdir mpoint/user1
> > > # useradd -d /mnt/mpoint/user1 user1     # may also use existing user
> > > # chown user1 mpoint/user1
> > > # quotacheck -v mpoint                   # initializes quota file
> > > # edquota user1
> > > ---- set soft block limit to 1000, hard limit to 4000 ----
> > > # edquota -t
> > > ---- set the grace periods to something small: 1minutes ---
> > > # quotaon mpoint
> > > # ## at this point "repquota -a" should show the quota for user1
> > > # su user1
> > > # cd
> > > # ## now we are in user1 home dir as user1
> > > # cat /dev/zero > file1
> > > loop2: warning, user block quota exceeded.
> > > loop2: write failed, user block limit reached.
> > > cat: write error: No space left on device
> > > --- now we wait till the grace period expires (repquota -a) ----
> > > # cat "" > otherfile
> > > loop2: write failed, user block quota exceeded too long.
> > > ---- and it will hang forever ----
> > > # ## /mnt/mpoint can still be accessed, but /mnt/mpoint/user1 can't
> > > 
> > > 
> > > I tested this on an -mm patchset kernel (2.6.13-rc5-mm1), but I
> > > discovered the bug in my server which runs plain 2.6.12 with the
> > > patch from Jeff Mahoney for the first reiserfs+acl bug.
> > > 
> > > The main difference between the two bugs is that the first one requires
> > > the existance of a default acl, this one does not, but it does require
> > > acl to be enabled.
> >   This seems to be the same problem as bug #4771 that I've just fix. Can
> > you try attached patch please?
> >   Andrew, can you include the patch into -mm if ReiserFS guys won't object?
> 
-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs

[-- Attachment #2: reiser-2.6.13-rc6-2-xattr_fix.diff --]
[-- Type: text/plain, Size: 956 bytes --]

When i_acl_default is set to some error we do not hold the lock (hence we are
not allowed to drop it and reacquire later).

Signed-off-by: Jan Kara <jack@suse.cz>

diff -rupX /home/jack/.kerndiffexclude linux-2.6.13-rc6-1-reiser_create_fix/fs/reiserfs/inode.c linux-2.6.13-rc6-2-reiser_xattr_fix/fs/reiserfs/inode.c
--- linux-2.6.13-rc6-1-reiser_create_fix/fs/reiserfs/inode.c	2005-08-14 17:10:21.000000000 +0200
+++ linux-2.6.13-rc6-2-reiser_xattr_fix/fs/reiserfs/inode.c	2005-08-14 17:11:35.000000000 +0200
@@ -1985,7 +1985,7 @@ int reiserfs_new_inode(struct reiserfs_t
 	 * iput doesn't deadlock in reiserfs_delete_xattrs. The locking
 	 * code really needs to be reworked, but this will take care of it
 	 * for now. -jeffm */
-	if (REISERFS_I(dir)->i_acl_default) {
+	if (REISERFS_I(dir)->i_acl_default && !IS_ERR(REISERFS_I(dir)->i_acl_default)) {
 		reiserfs_write_unlock_xattrs(dir->i_sb);
 		iput(inode);
 		reiserfs_write_lock_xattrs(dir->i_sb);