Fix kernel NULL pointer dereference by dlm_astd

Hi,

This patch fixes a problem in an error path when looking up dinodes. There are two sister-functions, gfs2_inode_lookup and gfs2_process_unlinked_inode. Both functions acquire and hold the i_iopen glock for the dinode being looked up. The last thing they try to do is hold the i_gl glock for the dinode. If that glock fails for some reason, the error path was incorrectly calling gfs2_glock_put for the i_iopen glock twice. This resulted in the glock being prematurely freed. The "minimum hold time" usually kept the glock in memory, but the lock interface to dlm (aka lock_dlm) freed its memory for the glock. In some circumstances, it would cause dlm's dlm_astd daemon to try to call the bast function for the freed lock_dlm memory, which resulted in a NULL pointer dereference.

This problem was discovered while testing bugzilla bug #595397.

Regards,

Bob Peterson
Red Hat GFS

--

 fs/gfs2/inode.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

index b5612cb..43e06ff 100644
@@ -197,8 +197,6 @@ struct inode *gfs2_inode_lookup(struct super_block *sb,
 			goto fail_iopen;
 		ip->i_iopen_gh.gh_gl->gl_object = ip;
 
-		gfs2_glock_put(io_gl);
-
 		if ((type == DT_UNKNOWN) && (no_formal_ino == 0))
 			goto gfs2_nfsbypass;
 

@@ -224,6 +222,8 @@ struct inode *gfs2_inode_lookup(struct super_block *sb,
 	}
 
 gfs2_nfsbypass:
+	gfs2_glock_put(io_gl);
+
 	return inode;
 fail_glock:
 	gfs2_glock_dq(&ip->i_iopen_gh);

@@ -292,7 +292,6 @@ void gfs2_process_unlinked_inode(struct super_block *sb, u64 no_addr)
 		goto fail_iopen;
 
 	ip->i_iopen_gh.gh_gl->gl_object = ip;
-	gfs2_glock_put(io_gl);
 
 	inode->i_mode = DT2IF(DT_UNKNOWN);
 

@@ -310,6 +309,7 @@ void gfs2_process_unlinked_inode(struct super_block *sb, u64 no_addr)
 
 	/* Inode is now uptodate */
 	gfs2_glock_dq_uninit(&gh);
+	gfs2_glock_put(io_gl);
 	gfs2_set_iop(inode);
 
 	/* The iput will cause it to be deleted. */

Hi,

Now in the -nmw GFS2 tree. Thanks,

Steve.

----- "Steven Whitehouse" <[email protected]> wrote:

| Hi,
| 
| Now in the -nmw GFS2 tree. Thanks,
| 
| Steve.
| 
| On Tue, 2010-06-15 at 12:07 -0400, Bob Peterson wrote:
| > Hi,
| > 
| > This patch fixes a problem in an error path when looking
| > up dinodes.  There are two sister-functions, gfs2_inode_lookup
| > and gfs2_process_unlinked_inode.  Both functions acquire and
| > hold the i_iopen glock for the dinode being looked up. The last
| > thing they try to do is hold the i_gl glock for the dinode.
| > If that glock fails for some reason, the error path was
| > incorrectly calling gfs2_glock_put for the i_iopen glock twice.
| > This resulted in the glock being prematurely freed.  The
| > "minimum hold time" usually kept the glock in memory, but the
| > lock interface to dlm (aka lock_dlm) freed its memory for the
| > glock.  In some circumstances, it would cause dlm's dlm_astd daemon
| > to try to call the bast function for the freed lock_dlm memory,
| > which resulted in a NULL pointer dereference.
| > 
| > This problem was discovered while testing bugzilla bug #595397.
| > 
| > Regards,
| > 
| > Bob Peterson
| > Red Hat GFS

Hi,

Actually, it's not yet in the -nmw git tree. I think Steve W. forgot to push it before he left on holiday. At any rate, that's a good thing because my testing has uncovered a possible problem with this patch. I'm planning to rework it and re-post when I get a stable version.

Regards,

Bob Peterson
Red Hat File Systems