On Thu, Apr 17, 2008 at 10:34:06AM -0500, Serge E. Hallyn wrote:
quoted text
> Quoting Michael Halcrow (mhalcrow@us.ibm.com):
> > @@ -206,6 +210,7 @@ ecryptfs_spawn_daemon(struct ecryptfs_daemon **daemon, uid_t euid, pid_t pid)
> >  		goto out;
> >  	}
> >  	(*daemon)->euid = euid;
> > +	(*daemon)->user_ns = user_ns;
> >  	(*daemon)->pid = pid;
> 
> You'll want to do a get_pid() here, no?
> 
> And get_user_ns().
> 

quoted text
> It's not because you particulary need them to stick around, but just
> to ensure no wraparound causes another daemon with the same struct
> pid or user_namespace to be spawned.  Pretty gosh-darned unlikely,
> but still...
> ...
> > @@ -372,12 +383,24 @@ int ecryptfs_process_response(struct ecryptfs_message *msg, uid_t euid,
> >  	msg_ctx = &ecryptfs_msg_ctx_arr[msg->index];
> >  	mutex_lock(&msg_ctx->mux);
> >  	mutex_lock(&ecryptfs_daemon_hash_mux);
> > -	rc = ecryptfs_find_daemon_by_euid(&daemon, msg_ctx->task->euid);
> > +	rcu_read_lock();
> > +	nsproxy = task_nsproxy(msg_ctx->task);
> > +	if (nsproxy == NULL) {
> > +		rc = -EBADMSG;
> > +		printk(KERN_ERR "%s: Receiving process is a zombie. Dropping "
> > +		       "message.\n", __func__);
> > +		rcu_read_unlock();
> > +		mutex_unlock(&ecryptfs_daemon_hash_mux);
> > +		goto wake_up;
> > +	}
> > +	rc = ecryptfs_find_daemon_by_euid(&daemon, msg_ctx->task->euid,
> > +					  nsproxy->user_ns);
> > +	rcu_read_unlock();
> >  	mutex_unlock(&ecryptfs_daemon_hash_mux);
> >  	if (rc) {
> >  		rc = -EBADMSG;
> >  		printk(KERN_WARNING "%s: User [%d] received a "
> > -		       "message response from process [%d] but does "
> > +		       "message response from process [0x%p] but does "
> >  		       "not have a registered daemon\n", __func__,
> >  		       msg_ctx->task->euid, pid);
> >  		goto wake_up;
> > @@ -389,10 +412,17 @@ int ecryptfs_process_response(struct ecryptfs_message *msg, uid_t euid,
> >  		       euid, msg_ctx->task->euid);
> >  		goto unlock;
> >  	}
> > +	if (nsproxy->user_ns != user_ns) {
> 
> Since you didn't grab a ref to the nsproxy, it's possible that it
> will have been freed before this, right?  So you probably just want
> to grab a copy of nsproxy->user_ns while under the rcu_read_lock,
> where you can be sure it's still around.

Have eCryptfs properly reference the pid and user_ns objects. Copy
user_ns out of nsproxy in case nsproxy goes away after we drop the
lock.

Signed-off-by: Michael Halcrow <mhalcrow@us.ibm.com>
---
 fs/ecryptfs/messaging.c |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/fs/ecryptfs/messaging.c b/fs/ecryptfs/messaging.c
index f0d74b8..61506e5 100644
--- a/fs/ecryptfs/messaging.c
+++ b/fs/ecryptfs/messaging.c
@@ -20,6 +20,8 @@
  * 02111-1307, USA.
  */
 #include <linux/sched.h>
+#include <linux/user_namespace.h>
+#include <linux/nsproxy.h>
 #include "ecryptfs_kernel.h"
 
 static LIST_HEAD(ecryptfs_msg_ctx_free_list);
@@ -208,8 +210,8 @@ ecryptfs_spawn_daemon(struct ecryptfs_daemon **daemon, uid_t euid,
 		goto out;
 	}
 	(*daemon)->euid = euid;
-	(*daemon)->user_ns = user_ns;
-	(*daemon)->pid = pid;
+	(*daemon)->user_ns = get_user_ns(user_ns);
+	(*daemon)->pid = get_pid(pid);
 	(*daemon)->task = current;
 	mutex_init(&(*daemon)->mux);
 	INIT_LIST_HEAD(&(*daemon)->msg_ctx_out_queue);
@@ -298,6 +300,10 @@ int ecryptfs_exorcise_daemon(struct ecryptfs_daemon *daemon)
 	hlist_del(&daemon->euid_chain);
 	if (daemon->task)
 		wake_up_process(daemon->task);
+	if (daemon->pid)
+		put_pid(daemon->pid);
+	if (daemon->user_ns)
+		put_user_ns(daemon->user_ns);
 	mutex_unlock(&daemon->mux);
 	memset(daemon, 0, sizeof(*daemon));
 	kfree(daemon);
@@ -368,6 +374,7 @@ int ecryptfs_process_response(struct ecryptfs_message *msg, uid_t euid,
 	struct ecryptfs_msg_ctx *msg_ctx;
 	size_t msg_size;
 	struct nsproxy *nsproxy;
+	struct user_namespace *current_user_ns;
 	int rc;
 
 	if (msg->index >= ecryptfs_message_buf_len) {
@@ -391,8 +398,9 @@ int ecryptfs_process_response(struct ecryptfs_message *msg, uid_t euid,
 		mutex_unlock(&ecryptfs_daemon_hash_mux);
 		goto wake_up;
 	}
+	current_user_ns = nsproxy->user_ns;
 	rc = ecryptfs_find_daemon_by_euid(&daemon, msg_ctx->task->euid,
-					  nsproxy->user_ns);
+					  current_user_ns);
 	rcu_read_unlock();
 	mutex_unlock(&ecryptfs_daemon_hash_mux);
 	if (rc) {
@@ -410,7 +418,7 @@ int ecryptfs_process_response(struct ecryptfs_message *msg, uid_t euid,
 		       euid, msg_ctx->task->euid);
 		goto unlock;
 	}
-	if (nsproxy->user_ns != user_ns) {
+	if (current_user_ns != user_ns) {
 		rc = -EBADMSG;
 		printk(KERN_WARNING "%s: Received message from user_ns "
 		       "[0x%p]; expected message from user_ns [0x%p]\n",
-- 
1.5.1.6

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