| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 21 May 2008 01:16:15 +0200
From: Andrea Righi <righiandr@...rs.sourceforge.net>
To: Michael Halcrow <mhalcrow@...ibm.com>
CC: Andrew Morton <akpm@...ux-foundation.org>,
LKML <linux-kernel@...r.kernel.org>,
Rusty Russell <prussell@....ibm.com>,
"David J. Kleikamp" <shaggy@...ibm.com>,
"SERGE E. HALLYN" <sergeh@...ibm.com>
Subject: Re: [PATCH] eCryptfs: Privileged kthread for lower file opens
Michael Halcrow wrote:
[snip]
Hi,
2 comments on the patch.
> diff --git a/fs/ecryptfs/kthread.c b/fs/ecryptfs/kthread.c
> new file mode 100644
> index 0000000..dba7c67
> --- /dev/null
> +++ b/fs/ecryptfs/kthread.c
> @@ -0,0 +1,211 @@
> +/**
> + * eCryptfs: Linux filesystem encryption layer
> + *
> + * Copyright (C) 2008 International Business Machines Corp.
> + * Author(s): Michael A. Halcrow <mahalcro@...ibm.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of the
> + * License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
> + * 02111-1307, USA.
> + */
> +
> +#include <linux/kthread.h>
> +#include <linux/freezer.h>
> +#include <linux/wait.h>
> +#include <linux/mount.h>
> +#include "ecryptfs_kernel.h"
> +
> +struct task_struct *ecryptfs_kthread;
> +struct ecryptfs_kthread_ctl ecryptfs_kthread_ctl;
> +
> +struct kmem_cache *ecryptfs_open_req_cache;
> +
> +/**
> + * ecryptfs_threadfn
> + * @ignored: ignored
> + *
> + * The eCryptfs kernel thread that has the responsibility of getting
> + * the lower persistent file with RW permissions.
> + *
> + * Returns zero on success; non-zero otherwise
> + */
> +static int ecryptfs_threadfn(void *ignored)
> +{
> + set_freezable();
> + while (1) {
> + struct ecryptfs_open_req *req;
> +
> + wait_event_freezable(
> + ecryptfs_kthread_ctl.wait,
> + !list_empty(&ecryptfs_kthread_ctl.req_list));
> + mutex_lock(&ecryptfs_kthread_ctl.mux);
> + if (ecryptfs_kthread_ctl.flags & ECRYPTFS_KTHREAD_ZOMBIE) {
> + mutex_unlock(&ecryptfs_kthread_ctl.mux);
> + goto out;
> + }
> + while (!list_empty(&ecryptfs_kthread_ctl.req_list)) {
> + req = list_first_entry(&ecryptfs_kthread_ctl.req_list,
> + struct ecryptfs_open_req,
> + kthread_ctl_list);
> + BUG_ON(!req);
> + mutex_lock(&req->mux);
> + list_del(&req->kthread_ctl_list);
> + if (req->flags & ECRYPTFS_REQ_ZOMBIE)
> + mutex_unlock(&req->mux);
> + else {
> + dget(req->lower_dentry);
> + mntget(req->lower_mnt);
> + (*req->lower_file) = dentry_open(
> + req->lower_dentry, req->lower_mnt,
> + (O_RDWR | O_LARGEFILE));
> + req->flags |= ECRYPTFS_REQ_PROCESSED;
> + wake_up_process(req->requesting_task);
> + mutex_unlock(&req->mux);
> + }
Why not:
if (!(req->flags & ECRYPTFS_REQ_ZOMBIE))
dget(req->lower_dentry);
mntget(req->lower_mnt);
(*req->lower_file) = dentry_open(
req->lower_dentry, req->lower_mnt,
(O_RDWR | O_LARGEFILE));
req->flags |= ECRYPTFS_REQ_PROCESSED;
wake_up_process(req->requesting_task);
}
mutex_unlock(&req->mux);
> + }
> + mutex_unlock(&ecryptfs_kthread_ctl.mux);
> + }
> +out:
> + do_exit(0);
> +}
> +
> +int ecryptfs_init_kthread(void)
> +{
> + int rc = 0;
> +
> + mutex_init(&ecryptfs_kthread_ctl.mux);
> + init_waitqueue_head(&ecryptfs_kthread_ctl.wait);
> + INIT_LIST_HEAD(&ecryptfs_kthread_ctl.req_list);
> + ecryptfs_kthread = kthread_create(&ecryptfs_threadfn, NULL,
> + "ecryptfs-kthread");
> + if (IS_ERR(ecryptfs_kthread)) {
> + rc = PTR_ERR(ecryptfs_kthread);
> + printk(KERN_ERR "%s: Failed to create kernel thread; rc = [%d]"
> + "\n", __func__, rc);
> + }
> + wake_up_process(ecryptfs_kthread);
> + return rc;
> +}
> +
> +void ecryptfs_destroy_kthread(void)
> +{
> + struct ecryptfs_open_req tmp_req;
> + struct ecryptfs_open_req *req;
> +
> + mutex_lock(&ecryptfs_kthread_ctl.mux);
> + ecryptfs_kthread_ctl.flags |= ECRYPTFS_KTHREAD_ZOMBIE;
> + list_for_each_entry(req, &ecryptfs_kthread_ctl.req_list,
> + kthread_ctl_list) {
> + mutex_lock(&req->mux);
> + req->flags |= ECRYPTFS_REQ_ZOMBIE;
> + wake_up_process(req->requesting_task);
> + mutex_unlock(&req->mux);
> + }
> + memset(&tmp_req, 0, sizeof(tmp_req));
> + tmp_req.flags = ECRYPTFS_REQ_ZOMBIE;
> + list_add_tail(&tmp_req.kthread_ctl_list,
> + &ecryptfs_kthread_ctl.req_list);
> + mutex_unlock(&ecryptfs_kthread_ctl.mux);
> + wake_up(&ecryptfs_kthread_ctl.wait);
> +}
> +
> +/**
> + * ecryptfs_privileged_open
> + * @lower_file: Result of dentry_open by root on lower dentry
> + * @lower_dentry: Lower dentry for file to open
> + * @lower_mnt: Lower vfsmount for file to open
> + *
> + * This function gets a r/w file opened againt the lower dentry.
> + *
> + * Returns zero on success; non-zero otherwise
> + */
> +int ecryptfs_privileged_open(struct file **lower_file,
> + struct dentry *lower_dentry,
> + struct vfsmount *lower_mnt)
> +{
> + struct ecryptfs_open_req *req;
> + int rc = 0;
> +
> + /* Corresponding dput() and mntput() are done when the
> + * persistent file is fput() when the eCryptfs inode is
> + * destroyed. */
> + dget(lower_dentry);
> + mntget(lower_mnt);
> + (*lower_file) = dentry_open(lower_dentry, lower_mnt,
> + (O_RDWR | O_LARGEFILE));
> + if (!IS_ERR(*lower_file))
> + goto out;
> + req = kmem_cache_alloc(ecryptfs_open_req_cache, GFP_KERNEL);
> + if (!req) {
> + rc = -ENOMEM;
> + goto out;
> + }
> + mutex_init(&req->mux);
> + req->lower_file = lower_file;
> + req->lower_dentry = lower_dentry;
> + req->lower_mnt = lower_mnt;
> + req->requesting_task = current;
> + req->flags = 0;
> + mutex_lock(&ecryptfs_kthread_ctl.mux);
> + mutex_lock(&req->mux);
> + if (ecryptfs_kthread_ctl.flags & ECRYPTFS_KTHREAD_ZOMBIE) {
> + rc = -EIO;
Isn't a mutex_unlock(&req->mux) missing here?
> + mutex_unlock(&ecryptfs_kthread_ctl.mux);
> + printk(KERN_ERR "%s: We are in the middle of shutting down; "
> + "aborting privileged request to open lower file\n",
> + __func__);
> + goto out_free;
> + }
> + list_add_tail(&req->kthread_ctl_list, &ecryptfs_kthread_ctl.req_list);
> + mutex_unlock(&req->mux);
> + mutex_unlock(&ecryptfs_kthread_ctl.mux);
> + wake_up(&ecryptfs_kthread_ctl.wait);
> +schedule:
> + set_current_state(TASK_INTERRUPTIBLE);
> + schedule();
> + mutex_lock(&req->mux);
> + if (req->flags == 0) {
> + mutex_unlock(&req->mux);
> + goto schedule;
> + }
> + set_current_state(TASK_RUNNING);
> + if (req->flags & ECRYPTFS_REQ_DROPPED
> + || req->flags & ECRYPTFS_REQ_ZOMBIE) {
> + rc = -EIO;
> + printk(KERN_WARNING "%s: Privileged open request dropped\n",
> + __func__);
> + goto out_unlock;
> + }
> + if (IS_ERR(*req->lower_file)) {
> + rc = PTR_ERR(*req->lower_file);
> + dget(lower_dentry);
> + mntget(lower_mnt);
> + (*lower_file) = dentry_open(lower_dentry, lower_mnt,
> + (O_RDONLY | O_LARGEFILE));
> + if (IS_ERR(*lower_file)) {
> + rc = PTR_ERR(*req->lower_file);
> + (*lower_file) = NULL;
> + printk(KERN_WARNING "%s: Error attempting privileged "
> + "open of lower file with either RW or RO "
> + "perms; rc = [%d]. Giving up.\n",
> + __func__, rc);
> + }
> + }
> +out_unlock:
> + mutex_unlock(&req->mux);
> +out_free:
> + kmem_cache_free(ecryptfs_open_req_cache, req);
> +out:
> + return rc;
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists