One more unlock missing in error case

One more unlock missing in error case

[Ulrich Drepper]

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

This patch is needed to in addition to the other unlocking fix which is
already applied.  It should be obvious that the system will deadlock in
case this isn't done.

(Andrew, I once again used a goto in the third error case because the
common code is again larger.)

Signed-Off-By: Ulrich Drepper <drepper@redhat.com>

--- fs/namei.c	2006-02-01 09:29:49.000000000 -0800
+++ fs/namei.c-new	2006-02-04 09:18:02.000000000 -0800
@@ -1096,6 +1096,7 @@

 		file = fget_light(dfd, &fput_needed);
 		if (!file) {
+			read_unlock(&current->fs->lock);
 			retval = -EBADF;
 			goto out_fail;
 		}
@@ -1104,15 +1105,15 @@

 		if (!S_ISDIR(dentry->d_inode->i_mode)) {
 			retval = -ENOTDIR;
+		unlock_fail:
 			fput_light(file, fput_needed);
+			read_unlock(&current->fs->lock);
 			goto out_fail;
 		}

 		retval = file_permission(file, MAY_EXEC);
-		if (retval) {
-			fput_light(file, fput_needed);
-			goto out_fail;
-		}
+		if (retval)
+			goto unlock_fail;

 		nd->mnt = mntget(file->f_vfsmnt);
 		nd->dentry = dget(dentry);


-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 251 bytes --]

Re: One more unlock missing in error case

[Andrew Morton]

Ulrich Drepper <drepper@redhat.com> wrote:
>
> This patch is needed to in addition to the other unlocking fix which is
>  already applied.  It should be obvious that the system will deadlock in
>  case this isn't done.

hmm.  How's about we undo that goto tangle while we're there?


--- devel/fs/namei.c~nameic-unlock-missing-in-error-case	2006-02-04 15:57:22.000000000 -0800
+++ devel-akpm/fs/namei.c	2006-02-04 16:07:56.000000000 -0800
@@ -1070,6 +1070,8 @@ static int fastcall do_path_lookup(int d
 				unsigned int flags, struct nameidata *nd)
 {
 	int retval = 0;
+	int fput_needed;
+	struct file *file;
 
 	nd->last_type = LAST_ROOT; /* if there are only slashes... */
 	nd->flags = flags;
@@ -1091,29 +1093,22 @@ static int fastcall do_path_lookup(int d
 		nd->mnt = mntget(current->fs->pwdmnt);
 		nd->dentry = dget(current->fs->pwd);
 	} else {
-		struct file *file;
-		int fput_needed;
 		struct dentry *dentry;
 
 		file = fget_light(dfd, &fput_needed);
-		if (!file) {
-			retval = -EBADF;
-			goto out_fail;
-		}
+		retval = -EBADF;
+		if (!file)
+			goto unlock_fail;
 
 		dentry = file->f_dentry;
 
-		if (!S_ISDIR(dentry->d_inode->i_mode)) {
-			retval = -ENOTDIR;
-			fput_light(file, fput_needed);
-			goto out_fail;
-		}
+		retval = -ENOTDIR;
+		if (!S_ISDIR(dentry->d_inode->i_mode))
+			goto fput_unlock_fail;
 
 		retval = file_permission(file, MAY_EXEC);
-		if (retval) {
-			fput_light(file, fput_needed);
-			goto out_fail;
-		}
+		if (retval)
+			goto fput_unlock_fail;
 
 		nd->mnt = mntget(file->f_vfsmnt);
 		nd->dentry = dget(dentry);
@@ -1127,7 +1122,12 @@ out:
 	if (unlikely(current->audit_context
 		     && nd && nd->dentry && nd->dentry->d_inode))
 		audit_inode(name, nd->dentry->d_inode, flags);
-out_fail:
+	return retval;
+
+fput_unlock_fail:
+	fput_light(file, fput_needed);
+unlock_fail:
+	read_unlock(&current->fs->lock);
 	return retval;
 }
 
_

Re: One more unlock missing in error case

[Ulrich Drepper]

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

Andrew Morton wrote:
> hmm.  How's about we undo that goto tangle while we're there?

Fine with me.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 251 bytes --]

Re: One more unlock missing in error case

[Adrian Bunk]

On Sat, Feb 04, 2006 at 04:08:30PM -0800, Andrew Morton wrote:
> Ulrich Drepper <drepper@redhat.com> wrote:
> >
> > This patch is needed to in addition to the other unlocking fix which is
> >  already applied.  It should be obvious that the system will deadlock in
> >  case this isn't done.
> 
> hmm.  How's about we undo that goto tangle while we're there?
>...

The fput_unlock_fail label change looks good, but IMHO moving of retval 
settings out of the if's makes the code less readable.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed