[PATCH 2.6.5-mm4] sys_access race fix

[Fabian Frederick <Fabian.Frederick@skynet.be>]

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

Andrew,

	I'm trying to remove the race in sys_access code.
AFAICS, fsuid is checked in "permission" level so I pushed real fsuid
capture forward.At that state, I can task_lock (which was impossible
before user_walk).Could you tell me if I can improve this one ?

Regards,
Fabian

[-- Attachment #2: open1.diff --]
[-- Type: text/x-patch, Size: 2191 bytes --]

diff -Naur orig/fs/open.c edited/fs/open.c
--- orig/fs/open.c	2004-04-12 19:52:14.000000000 +0200
+++ edited/fs/open.c	2004-04-13 20:30:07.000000000 +0200
@@ -463,8 +463,8 @@
 
 /*
  * access() needs to use the real uid/gid, not the effective uid/gid.
- * We do this by temporarily clearing all FS-related capabilities and
- * switching the fsuid/fsgid around to the real ones.
+ * We do this under task_lock by temporarily clearing all FS-related 
+ * capabilities and switching the fsuid/fsgid around to the real ones.
  */
 asmlinkage long sys_access(const char __user * filename, int mode)
 {
@@ -476,40 +476,31 @@
 	if (mode & ~S_IRWXO)	/* where's F_OK, X_OK, W_OK, R_OK? */
 		return -EINVAL;
 
-	old_fsuid = current->fsuid;
-	old_fsgid = current->fsgid;
-	old_cap = current->cap_effective;
-
-	current->fsuid = current->uid;
-	current->fsgid = current->gid;
-
-	/*
-	 * Clear the capabilities if we switch to a non-root user
-	 *
-	 * FIXME: There is a race here against sys_capset.  The
-	 * capabilities can change yet we will restore the old
-	 * value below.  We should hold task_capabilities_lock,
-	 * but we cannot because user_path_walk can sleep.
-	 */
-	if (current->uid)
-		cap_clear(current->cap_effective);
-	else
-		current->cap_effective = current->cap_permitted;
-
 	res = __user_walk(filename, LOOKUP_FOLLOW|LOOKUP_ACCESS, &nd);
 	if (!res) {
+		task_lock(current);
+		old_fsuid = current->fsuid;
+		old_fsgid = current->fsgid;
+		old_cap = current->cap_effective;
+		current->fsuid = current->uid;
+		current->fsgid = current->gid;
+
+		if (current->uid)
+			cap_clear(current->cap_effective);
+		else
+			current->cap_effective = current->cap_permitted;
+
 		res = permission(nd.dentry->d_inode, mode, &nd);
 		/* SuS v2 requires we report a read only fs too */
 		if(!res && (mode & S_IWOTH) && IS_RDONLY(nd.dentry->d_inode)
 		   && !special_file(nd.dentry->d_inode->i_mode))
 			res = -EROFS;
 		path_release(&nd);
+		current->fsuid = old_fsuid;
+		current->fsgid = old_fsgid;
+		current->cap_effective = old_cap;
+		task_unlock(current);
 	}
-
-	current->fsuid = old_fsuid;
-	current->fsgid = old_fsgid;
-	current->cap_effective = old_cap;
-
 	return res;
 }