Re: [ANNOUNCE] Linux-2.4.32-hf32.5

[Marcelo Tosatti <marcelo@kvack.org>]

On Fri, May 26, 2006 at 04:07:31PM +0200, Willy Tarreau wrote:
> [ I removed Jari who told me yesterday he did not need to be Cc'd ]
> 
> On Fri, May 26, 2006 at 11:28:51PM +1000, Grant Coady wrote:
> > On Fri, 26 May 2006 14:16:23 +0200, Willy Tarreau <willy@w.ods.org> wrote:
> > 
> > >Could you please pass it through ksymoops so that we get an idea about the
> > >function causing this ? What was the last version not causing it ? hf32.4 ?
> > 
> > Yes, hf32.4 okay, see: <http://bugsplatter.mine.nu/test/linux-2.4/>
> > 
> > >This looks like a structure member gets accessed while a pointer is NULL,
> > >if you always get 0x88... I would be it could come from
> > >2.4.32-ext3-link-unlink-race-1, but that would be strange.
> > 
> > Good guess!  The previous version comment stripped .configs are 
> > linked by machine name from the summary page above.
> 
> Hmmm that's bad, this one has been merged into mainline.
> It would look like dentry->d_inode is NULL here :
> 
>   double_down(&dir->i_zombie, &dentry->d_inode->i_zombie);
> 
> I don't know how this can be fixed, though ! My first guess would be to
> quickly revert the patch. 

may_delete() should be called before attempting to grab victim's
i_zombie. Grant, can you please try the following?

diff --git a/fs/namei.c b/fs/namei.c
index 48bd26c..42cce98 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1479,19 +1479,20 @@ int vfs_unlink(struct inode *dir, struct
 {
 	int error;
 
-	double_down(&dir->i_zombie, &dentry->d_inode->i_zombie);
 	error = may_delete(dir, dentry, 0);
-	if (!error) {
-		error = -EPERM;
-		if (dir->i_op && dir->i_op->unlink) {
-			DQUOT_INIT(dir);
-			if (d_mountpoint(dentry))
-				error = -EBUSY;
-			else {
-				lock_kernel();
-				error = dir->i_op->unlink(dir, dentry);
-				unlock_kernel();
-			}
+	if (error)
+		return error;
+
+	double_down(&dir->i_zombie, &dentry->d_inode->i_zombie);
+	error = -EPERM;
+	if (dir->i_op && dir->i_op->unlink) {
+		DQUOT_INIT(dir);
+		if (d_mountpoint(dentry))
+			error = -EBUSY;
+		else {
+			lock_kernel();
+			error = dir->i_op->unlink(dir, dentry);
+			unlock_kernel();
 		}
 	}
 	double_up(&dir->i_zombie, &dentry->d_inode->i_zombie);