Commit 8dc67805 authored by Tyler Hicks's avatar Tyler Hicks

eCryptfs: Gracefully refuse miscdev file ops on inherited/passed files

File operations on /dev/ecryptfs would BUG() when the operations were
performed by processes other than the process that originally opened the
file. This could happen with open files inherited after fork() or file
descriptors passed through IPC mechanisms. Rather than calling BUG(), an
error code can be safely returned in most situations.

In ecryptfs_miscdev_release(), eCryptfs still needs to handle the
release even if the last file reference is being held by a process that
didn't originally open the file. ecryptfs_find_daemon_by_euid() will not
be successful, so a pointer to the daemon is stored in the file's
private_data. The private_data pointer is initialized when the miscdev
file is opened and only used when the file is released.

https://launchpad.net/bugs/994247Signed-off-by: default avatarTyler Hicks <tyhicks@canonical.com>
Reported-by: default avatarSasha Levin <levinsasha928@gmail.com>
Tested-by: default avatarSasha Levin <levinsasha928@gmail.com>
parent 60d65f1f
...@@ -49,7 +49,10 @@ ecryptfs_miscdev_poll(struct file *file, poll_table *pt) ...@@ -49,7 +49,10 @@ ecryptfs_miscdev_poll(struct file *file, poll_table *pt)
mutex_lock(&ecryptfs_daemon_hash_mux); mutex_lock(&ecryptfs_daemon_hash_mux);
/* TODO: Just use file->private_data? */ /* TODO: Just use file->private_data? */
rc = ecryptfs_find_daemon_by_euid(&daemon, euid, current_user_ns()); rc = ecryptfs_find_daemon_by_euid(&daemon, euid, current_user_ns());
BUG_ON(rc || !daemon); if (rc || !daemon) {
mutex_unlock(&ecryptfs_daemon_hash_mux);
return -EINVAL;
}
mutex_lock(&daemon->mux); mutex_lock(&daemon->mux);
mutex_unlock(&ecryptfs_daemon_hash_mux); mutex_unlock(&ecryptfs_daemon_hash_mux);
if (daemon->flags & ECRYPTFS_DAEMON_ZOMBIE) { if (daemon->flags & ECRYPTFS_DAEMON_ZOMBIE) {
...@@ -122,6 +125,7 @@ ecryptfs_miscdev_open(struct inode *inode, struct file *file) ...@@ -122,6 +125,7 @@ ecryptfs_miscdev_open(struct inode *inode, struct file *file)
goto out_unlock_daemon; goto out_unlock_daemon;
} }
daemon->flags |= ECRYPTFS_DAEMON_MISCDEV_OPEN; daemon->flags |= ECRYPTFS_DAEMON_MISCDEV_OPEN;
file->private_data = daemon;
atomic_inc(&ecryptfs_num_miscdev_opens); atomic_inc(&ecryptfs_num_miscdev_opens);
out_unlock_daemon: out_unlock_daemon:
mutex_unlock(&daemon->mux); mutex_unlock(&daemon->mux);
...@@ -152,9 +156,9 @@ ecryptfs_miscdev_release(struct inode *inode, struct file *file) ...@@ -152,9 +156,9 @@ ecryptfs_miscdev_release(struct inode *inode, struct file *file)
mutex_lock(&ecryptfs_daemon_hash_mux); mutex_lock(&ecryptfs_daemon_hash_mux);
rc = ecryptfs_find_daemon_by_euid(&daemon, euid, current_user_ns()); rc = ecryptfs_find_daemon_by_euid(&daemon, euid, current_user_ns());
BUG_ON(rc || !daemon); if (rc || !daemon)
daemon = file->private_data;
mutex_lock(&daemon->mux); mutex_lock(&daemon->mux);
BUG_ON(daemon->pid != task_pid(current));
BUG_ON(!(daemon->flags & ECRYPTFS_DAEMON_MISCDEV_OPEN)); BUG_ON(!(daemon->flags & ECRYPTFS_DAEMON_MISCDEV_OPEN));
daemon->flags &= ~ECRYPTFS_DAEMON_MISCDEV_OPEN; daemon->flags &= ~ECRYPTFS_DAEMON_MISCDEV_OPEN;
atomic_dec(&ecryptfs_num_miscdev_opens); atomic_dec(&ecryptfs_num_miscdev_opens);
...@@ -270,8 +274,16 @@ ecryptfs_miscdev_read(struct file *file, char __user *buf, size_t count, ...@@ -270,8 +274,16 @@ ecryptfs_miscdev_read(struct file *file, char __user *buf, size_t count,
mutex_lock(&ecryptfs_daemon_hash_mux); mutex_lock(&ecryptfs_daemon_hash_mux);
/* TODO: Just use file->private_data? */ /* TODO: Just use file->private_data? */
rc = ecryptfs_find_daemon_by_euid(&daemon, euid, current_user_ns()); rc = ecryptfs_find_daemon_by_euid(&daemon, euid, current_user_ns());
BUG_ON(rc || !daemon); if (rc || !daemon) {
mutex_unlock(&ecryptfs_daemon_hash_mux);
return -EINVAL;
}
mutex_lock(&daemon->mux); mutex_lock(&daemon->mux);
if (task_pid(current) != daemon->pid) {
mutex_unlock(&daemon->mux);
mutex_unlock(&ecryptfs_daemon_hash_mux);
return -EPERM;
}
if (daemon->flags & ECRYPTFS_DAEMON_ZOMBIE) { if (daemon->flags & ECRYPTFS_DAEMON_ZOMBIE) {
rc = 0; rc = 0;
mutex_unlock(&ecryptfs_daemon_hash_mux); mutex_unlock(&ecryptfs_daemon_hash_mux);
...@@ -308,9 +320,6 @@ ecryptfs_miscdev_read(struct file *file, char __user *buf, size_t count, ...@@ -308,9 +320,6 @@ ecryptfs_miscdev_read(struct file *file, char __user *buf, size_t count,
* message from the queue; try again */ * message from the queue; try again */
goto check_list; goto check_list;
} }
BUG_ON(euid != daemon->euid);
BUG_ON(current_user_ns() != daemon->user_ns);
BUG_ON(task_pid(current) != daemon->pid);
msg_ctx = list_first_entry(&daemon->msg_ctx_out_queue, msg_ctx = list_first_entry(&daemon->msg_ctx_out_queue,
struct ecryptfs_msg_ctx, daemon_out_list); struct ecryptfs_msg_ctx, daemon_out_list);
BUG_ON(!msg_ctx); BUG_ON(!msg_ctx);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment