Re: [PATCH] Call security hook from pid*_revalidate

[Stephen Smalley]

On Wed, 2003-08-20 at 10:56, OGAWA Hirofumi wrote:
> Umm.. Wasn't the following needed?
> 
> 	inode->i_uid = 0;
> 	inode->i_gid = 0;
> 	if (ino == PROC_PID_INO || task_dumpable(task)) {

Yes, good catch.  That's an omission from the original
proc-pid-setuid-ownership-fix.patch, not mine.  How about the patch
below, against 2.6.0-test3-mm3, which both addresses the omission from
the original patch and adds the security hook call.  Note that the
security hook call is still unconditional, as in proc_pid_make_inode;
the security module can decide how it wants to deal with non-dumpable
tasks when setting the inode security field.

diff -X /home/sds/dontdiff -ru linux-2.6.0-test3-mm3/fs/proc/base.c linux-2.6.0-test3-mm3-sds/fs/proc/base.c
--- linux-2.6.0-test3-mm3/fs/proc/base.c	2003-08-21 16:24:17.469096884 -0400
+++ linux-2.6.0-test3-mm3-sds/fs/proc/base.c	2003-08-21 16:09:40.000000000 -0400
@@ -870,11 +870,17 @@
  */
 static int pid_revalidate(struct dentry *dentry, struct nameidata *nd)
 {
-	if (pid_alive(proc_task(dentry->d_inode))) {
-		struct task_struct *task = proc_task(dentry->d_inode);
-
-		dentry->d_inode->i_uid = task->euid;
-		dentry->d_inode->i_gid = task->egid;
+	struct inode *inode = dentry->d_inode;
+	struct task_struct *task = proc_task(inode);
+	if (pid_alive(task)) {
+		int ino = inode->i_ino & 0xffff;
+		inode->i_uid = 0;
+		inode->i_gid = 0;
+		if (ino == PROC_PID_INO || task_dumpable(task)) {
+			inode->i_uid = task->euid;
+			inode->i_gid = task->egid;
+		}
+		security_task_to_inode(task, inode);
 		return 1;
 	}
 	d_drop(dentry);
@@ -883,8 +889,9 @@
 
 static int pid_fd_revalidate(struct dentry *dentry, struct nameidata *nd)
 {
-	struct task_struct *task = proc_task(dentry->d_inode);
-	int fd = proc_type(dentry->d_inode) - PROC_PID_FD_DIR;
+	struct inode *inode = dentry->d_inode;
+	struct task_struct *task = proc_task(inode);
+	int fd = proc_type(inode) - PROC_PID_FD_DIR;
 	struct files_struct *files;
 
 	task_lock(task);
@@ -895,10 +902,16 @@
 	if (files) {
 		spin_lock(&files->file_lock);
 		if (fcheck_files(files, fd)) {
+			int ino = inode->i_ino & 0xffff;
 			spin_unlock(&files->file_lock);
 			put_files_struct(files);
-			dentry->d_inode->i_uid = task->euid;
-			dentry->d_inode->i_gid = task->egid;
+			inode->i_uid = 0;
+			inode->i_gid = 0;
+			if (ino == PROC_PID_INO || task_dumpable(task)) {
+				inode->i_uid = task->euid;
+				inode->i_gid = task->egid;
+			}
+			security_task_to_inode(task, inode);
 			return 1;
 		}
 		spin_unlock(&files->file_lock);


-- 
Stephen Smalley <sds@xxxxxxxxxxxxxx>
National Security Agency

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/