OpenVZ Forum - RDF feed
type: channel
items: Re: [PATCH] eCryptfs: Make key module subsystem respect namespaces
items: [PATCH] eCryptfs: Fix refs to pid and user_ns

Re: [PATCH] eCryptfs: Make key module subsystem respect namespaces
type: item
description: Quoting Michael Halcrow (mhalcrow@us.ibm.com): > On Tue, Apr 15, 2008 at 04:34:02PM -0500, Serge E. Hallyn wrote: > > Quoting Andrew Morton (akpm@linux-foundation.org): > > > On Tue, 15 Apr 2008 15:23:13 -0500 > > > Michael Halcrow <mhalcrow@us.ibm.com> wrote: > > > > > > > ... > > > > + rc = ecryptfs_find_daemon_by_euid(&daemon, current->euid); > > > > + if (daemon->pid != current->pid) { > > > > + rc = ecryptfs_find_daemon_by_euid(&daemon, current->euid); > > > > + BUG_ON(current->euid != daemon->euid); > > > > + BUG_ON(current->pid != daemon->pid); > > > > > > This code uses pids and uids all over the place. Will it operate > > > correctly in a containerised environment? > > > > Thanks Andrew. > > > > Mike, the pid_t definately needs to be replaced with a struct pid. > > > > As for the euid, it'd be best if you also compared the user_namespace * > > to make sure we support one ecryptfs deamon per user namespace. > > Make eCryptfs key module subsystem respect namespaces. > > Since I will be removing the netlink interface in a future patch, I > just made changes to the netlink.c code so that it will not break the > build. With my recent patches, the kernel module currently defaults to > the device handle interface rather than the netlink interface. > > Signed-off-by: Michael Halcrow <mhalcrow@us.ibm.com> > --- > fs/ecryptfs/ecryptfs_kernel.h | 25 +++++++++----- > fs/ecryptfs/messaging.c | 71 +++++++++++++++++++++++++++++----------- > fs/ecryptfs/miscdev.c | 68 +++++++++++++++++++++++++-------------- > fs/ecryptfs/netlink.c | 25 +++++++++----- > 4 files changed, 126 insertions(+), 63 deletions(-) > > diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h > index 88b85f7..dc11431 100644 > --- a/fs/ecryptfs/ecryptfs_kernel.h > +++ b/fs/ecryptfs/ecryptfs_kernel.h > @@ -34,6 +34,7 @@ > #include <linux/namei.h> > #include <linux/scatterlist.h> > #include <linux/hash.h> > +#include <linux/nsproxy.h> > > /* Version verification for shared data structure…
creator: serue
date: 2008-04-17T15:34:06-00:00

[PATCH] eCryptfs: Fix refs to pid and user_ns
type: item
description: On Thu, Apr 17, 2008 at 10:34:06AM -0500, Serge E. Hallyn wrote: > 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(). > > 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 y…
creator: Michael Halcrow
date: 2008-04-17T17:03:31-00:00