* ecryptfs: kernel BUG at fs/ecryptfs/miscdev.c:52
@ 2012-05-26 19:39 Sasha Levin
2012-06-07 0:41 ` Sasha Levin
2012-06-13 0:05 ` [PATCH 0/3] eCryptfs: Fix and simplify messaging code Tyler Hicks
0 siblings, 2 replies; 8+ messages in thread
From: Sasha Levin @ 2012-05-26 19:39 UTC (permalink / raw)
To: tyhicks, dustin.kirkland; +Cc: ecryptfs, linux-kernel@vger.kernel.org
Hi all,
During fuzzing with trinity inside a KVM guest, using latest linux-next kernel, I've stumbled on the following:
[ 175.995560] ------------[ cut here ]------------
[ 175.996026] kernel BUG at fs/ecryptfs/miscdev.c:52!
[ 175.996026] invalid opcode: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
[ 175.996026] CPU 3
[ 175.996026] Pid: 7903, comm: trinity-child3 Tainted: G W 3.4.0-next-20120524-sasha #296 Bochs Bochs
[ 175.996026] RIP: 0010:[<ffffffff813e945a>] [<ffffffff813e945a>] ecryptfs_miscdev_poll+0x5a/0x150
[ 175.996026] RSP: 0018:ffff88002d4a3aa8 EFLAGS: 00010282
[ 175.996026] RAX: 00000000ffffffea RBX: ffff88002d4a3b98 RCX: ffff880019d3c348
[ 175.996026] RDX: ffffffff84444140 RSI: 000000000b32e681 RDI: ffff88002d4a3ab0
[ 175.996026] RBP: ffff88002d4a3ad8 R08: cbf7d334712ee681 R09: 0200000000000000
[ 175.996026] R10: 2000000000000000 R11: 3408000000000000 R12: ffff88003d53c000
[ 176.022035] R13: 000000000b32e681 R14: ffff88002d4b059c R15: ffff88003d53c000
165983 iteration[ 176.026020] FS: 00007f0abac73700(0000) GS:ffff880035a00000(0000) knlGS:0000000000000000
[ 176.026020] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
s.
[ 176.026020] CR2: 00000000011750d8 CR3: 0000000039722000 CR4: 00000000000407e0
[ 176.026020] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 176.026020] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 176.026020] Process trinity-child3 (pid: 7903, threadinfo ffff88002d4a2000, task ffff88002d503000)
[ 176.026020] Stack:
[ 176.026020] ffff88002d4b069c ffff880019d3c348 ffff88002d4b069c ffff88002d4b069c
[ 176.026020] ffff88002d4a3b44 ffff88002d4a3b98 ffff88002d4a3b78 ffffffff8124d9bc
[ 176.026020] ffff88002d503000 ffff88002d503000 01ffffff811f5afe 0000000000000000
[ 176.026020] Call Trace:
[ 176.026020] [<ffffffff8124d9bc>] do_poll+0x11c/0x2b0
[ 176.026020] [<ffffffff8124dcbf>] do_sys_poll+0x16f/0x240
[ 176.026020] [<ffffffff8114dfdd>] ? __lock_acquired+0x3d/0x2e0
[ 176.026020] [<ffffffff811fade6>] ? do_wp_page+0x5c6/0x800
[ 176.026020] [<ffffffff811faf73>] ? do_wp_page+0x753/0x800
197143 iteration[ 176.026020] [<ffffffff8197e060>] ? do_raw_spin_unlock+0xd0/0xe0
s.
[ 176.026020] [<ffffffff83249080>] ? _raw_spin_unlock+0x30/0x60
[ 176.026020] [<ffffffff811fafe8>] ? do_wp_page+0x7c8/0x800
[ 176.026020] [<ffffffff8114dfdd>] ? __lock_acquired+0x3d/0x2e0
[ 176.026020] [<ffffffff8198044d>] ? debug_object_activate+0x6d/0x1b0
[ 176.026020] [<ffffffff81980532>] ? debug_object_activate+0x152/0x1b0
[ 176.026020] [<ffffffff8197e060>] ? do_raw_spin_unlock+0xd0/0xe0
[ 176.026020] [<ffffffff8114b4cd>] ? trace_hardirqs_off+0xd/0x10
[ 176.026020] [<ffffffff83249144>] ? _raw_spin_unlock_irqrestore+0x94/0xc0
[ 176.026020] [<ffffffff81980532>] ? debug_object_activate+0x152/0x1b0
[ 176.026020] [<ffffffff819611ee>] ? rb_insert_color+0x9e/0x160
[ 176.026020] [<ffffffff8110c488>] ? __hrtimer_start_range_ns+0x448/0x490
[ 176.026020] [<ffffffff8197e060>] ? do_raw_spin_unlock+0xd0/0xe0
[ 176.026020] [<ffffffff8124df6c>] sys_poll+0x6c/0x100
[ 176.026020] [<ffffffff8324a1f9>] system_call_fastpath+0x16/0x1b
[ 176.026020] Code: 00 00 44 8b 68 24 e8 66 cc e5 01 48 8d 7d d8 48 c7 c2 40 41 44 84 44 89 ee e8 a3 ef ff ff 85 c0 75 09 48 8b 7d
d8 48 85 ff 75 0e <0f> 0b 0f 1f 40 00 eb fe 66 0f 1f 44 00 00 48 83 c7 28 31 f6 e8
[ 176.026020] RIP [<ffffffff813e945a>] ecryptfs_miscdev_poll+0x5a/0x150
[ 176.026020] RSP <ffff88002d4a3aa8>
[ 176.028658] ---[ end trace 8f6ca168297608bd ]---
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: ecryptfs: kernel BUG at fs/ecryptfs/miscdev.c:52
2012-05-26 19:39 ecryptfs: kernel BUG at fs/ecryptfs/miscdev.c:52 Sasha Levin
@ 2012-06-07 0:41 ` Sasha Levin
2012-06-13 0:05 ` [PATCH 0/3] eCryptfs: Fix and simplify messaging code Tyler Hicks
1 sibling, 0 replies; 8+ messages in thread
From: Sasha Levin @ 2012-06-07 0:41 UTC (permalink / raw)
To: tyhicks; +Cc: dustin.kirkland, ecryptfs, linux-kernel@vger.kernel.org
On Sat, 2012-05-26 at 21:39 +0200, Sasha Levin wrote:
> Hi all,
>
> During fuzzing with trinity inside a KVM guest, using latest linux-next kernel, I've stumbled on the following:
>
> [ 175.995560] ------------[ cut here ]------------
> [ 175.996026] kernel BUG at fs/ecryptfs/miscdev.c:52!
> [ 175.996026] invalid opcode: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> [ 175.996026] CPU 3
> [ 175.996026] Pid: 7903, comm: trinity-child3 Tainted: G W 3.4.0-next-20120524-sasha #296 Bochs Bochs
> [ 175.996026] RIP: 0010:[<ffffffff813e945a>] [<ffffffff813e945a>] ecryptfs_miscdev_poll+0x5a/0x150
> [ 175.996026] RSP: 0018:ffff88002d4a3aa8 EFLAGS: 00010282
> [ 175.996026] RAX: 00000000ffffffea RBX: ffff88002d4a3b98 RCX: ffff880019d3c348
> [ 175.996026] RDX: ffffffff84444140 RSI: 000000000b32e681 RDI: ffff88002d4a3ab0
> [ 175.996026] RBP: ffff88002d4a3ad8 R08: cbf7d334712ee681 R09: 0200000000000000
> [ 175.996026] R10: 2000000000000000 R11: 3408000000000000 R12: ffff88003d53c000
> [ 176.022035] R13: 000000000b32e681 R14: ffff88002d4b059c R15: ffff88003d53c000
> 165983 iteration[ 176.026020] FS: 00007f0abac73700(0000) GS:ffff880035a00000(0000) knlGS:0000000000000000
> [ 176.026020] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> s.
> [ 176.026020] CR2: 00000000011750d8 CR3: 0000000039722000 CR4: 00000000000407e0
> [ 176.026020] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 176.026020] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [ 176.026020] Process trinity-child3 (pid: 7903, threadinfo ffff88002d4a2000, task ffff88002d503000)
> [ 176.026020] Stack:
> [ 176.026020] ffff88002d4b069c ffff880019d3c348 ffff88002d4b069c ffff88002d4b069c
> [ 176.026020] ffff88002d4a3b44 ffff88002d4a3b98 ffff88002d4a3b78 ffffffff8124d9bc
> [ 176.026020] ffff88002d503000 ffff88002d503000 01ffffff811f5afe 0000000000000000
> [ 176.026020] Call Trace:
> [ 176.026020] [<ffffffff8124d9bc>] do_poll+0x11c/0x2b0
> [ 176.026020] [<ffffffff8124dcbf>] do_sys_poll+0x16f/0x240
> [ 176.026020] [<ffffffff8114dfdd>] ? __lock_acquired+0x3d/0x2e0
> [ 176.026020] [<ffffffff811fade6>] ? do_wp_page+0x5c6/0x800
> [ 176.026020] [<ffffffff811faf73>] ? do_wp_page+0x753/0x800
> 197143 iteration[ 176.026020] [<ffffffff8197e060>] ? do_raw_spin_unlock+0xd0/0xe0
> s.
> [ 176.026020] [<ffffffff83249080>] ? _raw_spin_unlock+0x30/0x60
> [ 176.026020] [<ffffffff811fafe8>] ? do_wp_page+0x7c8/0x800
> [ 176.026020] [<ffffffff8114dfdd>] ? __lock_acquired+0x3d/0x2e0
> [ 176.026020] [<ffffffff8198044d>] ? debug_object_activate+0x6d/0x1b0
> [ 176.026020] [<ffffffff81980532>] ? debug_object_activate+0x152/0x1b0
> [ 176.026020] [<ffffffff8197e060>] ? do_raw_spin_unlock+0xd0/0xe0
> [ 176.026020] [<ffffffff8114b4cd>] ? trace_hardirqs_off+0xd/0x10
> [ 176.026020] [<ffffffff83249144>] ? _raw_spin_unlock_irqrestore+0x94/0xc0
> [ 176.026020] [<ffffffff81980532>] ? debug_object_activate+0x152/0x1b0
> [ 176.026020] [<ffffffff819611ee>] ? rb_insert_color+0x9e/0x160
> [ 176.026020] [<ffffffff8110c488>] ? __hrtimer_start_range_ns+0x448/0x490
> [ 176.026020] [<ffffffff8197e060>] ? do_raw_spin_unlock+0xd0/0xe0
> [ 176.026020] [<ffffffff8124df6c>] sys_poll+0x6c/0x100
> [ 176.026020] [<ffffffff8324a1f9>] system_call_fastpath+0x16/0x1b
> [ 176.026020] Code: 00 00 44 8b 68 24 e8 66 cc e5 01 48 8d 7d d8 48 c7 c2 40 41 44 84 44 89 ee e8 a3 ef ff ff 85 c0 75 09 48 8b 7d
> d8 48 85 ff 75 0e <0f> 0b 0f 1f 40 00 eb fe 66 0f 1f 44 00 00 48 83 c7 28 31 f6 e8
> [ 176.026020] RIP [<ffffffff813e945a>] ecryptfs_miscdev_poll+0x5a/0x150
> [ 176.026020] RSP <ffff88002d4a3aa8>
> [ 176.028658] ---[ end trace 8f6ca168297608bd ]---
>
I'm also seeing this, which is probably related:
[ 269.149993] ------------[ cut here ]------------
[ 269.150735] kernel BUG at fs/ecryptfs/miscdev.c:272!
[ 269.150735] invalid opcode: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
[ 269.150735] CPU 0
[ 269.150735] Pid: 8059, comm: trinity-child0 Tainted: G W 3.5.0-rc1-next-20120605-sasha-00012-g1389a60 #355
[ 269.150735] RIP: 0010:[<ffffffff813eb711>] [<ffffffff813eb711>] ecryptfs_miscdev_read+0x61/0x470
[ 269.150735] RSP: 0018:ffff88000baf3e58 EFLAGS: 00010282
[ 269.150735] RAX: 00000000ffffffea RBX: 00000000ffffffea RCX: ffff880028d9c620
[ 269.150735] RDX: ffffffff84a4d140 RSI: 000000007184449d RDI: ffff88000baf3eb0
[ 269.150735] RBP: ffff88000baf3ef8 R08: db1639ef5f10449d R09: 3a00000000000000
[ 269.150735] R10: a000000000000000 R11: 24e8000000000000 R12: ffff88000bb1b000
[ 269.150735] R13: 0000000000f59af0 R14: 0000000000000000 R15: 000000007184449d
[ 269.150735] FS: 00007fa2c283e700(0000) GS:ffff88000d800000(0000) knlGS:0000000000000000
[ 269.150735] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 269.150735] CR2: 00000000012d0098 CR3: 000000003349e000 CR4: 00000000000406f0
[ 269.150735] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 269.150735] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 269.150735] Process trinity-child0 (pid: 8059, threadinfo ffff88000baf2000, task ffff88000bb1b000)
[ 269.150735] Stack:
[ 269.150735] 0000000000000000 0000000000000000 ffff88000bb1b000 0000000000020000
[ 269.150735] ffff880019fbac10 ffff88001adeabe8 0000000000000004 0000000000000191
[ 269.150735] ffff88000baf3ec8 ffffffff818ae971 0000000000000000 ffff880019fbac00
[ 269.150735] Call Trace:
[ 269.150735] [<ffffffff818ae971>] ? security_file_permission+0x81/0x90
[ 269.150735] [<ffffffff81239e87>] vfs_read+0xc7/0x190
[ 269.150735] [<ffffffff8123a02f>] sys_read+0x4f/0x90
[ 269.150735] [<ffffffff837cdf39>] system_call_fastpath+0x16/0x1b
[ 269.150735] Code: 80 b5 fc 85 e8 51 e8 3d 02 48 8d 7d b8 48 c7 c2 40 d1 a4 84 44 89 fe e8 2e ea ff ff 89 c3 85 c0 75 09 48 8b 7d
b8 48 85 ff 75 0f <0f> 0b 0f 1f 44 00 00 eb fe 66 0f 1f 44 00 00 48 83 c7 28 31 f6
[ 269.150735] RIP [<ffffffff813eb711>] ecryptfs_miscdev_read+0x61/0x470
[ 269.150735] RSP <ffff88000baf3e58>
[ 269.188128] ---[ end trace 6d450e935ee18981 ]---
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 0/3] eCryptfs: Fix and simplify messaging code
2012-05-26 19:39 ecryptfs: kernel BUG at fs/ecryptfs/miscdev.c:52 Sasha Levin
2012-06-07 0:41 ` Sasha Levin
@ 2012-06-13 0:05 ` Tyler Hicks
2012-06-13 0:05 ` [PATCH 1/3] eCryptfs: Gracefully refuse miscdev file ops on inherited/passed files Tyler Hicks
` (2 more replies)
1 sibling, 3 replies; 8+ messages in thread
From: Tyler Hicks @ 2012-06-13 0:05 UTC (permalink / raw)
To: ecryptfs; +Cc: linux-kernel, Sasha Levin
Sasha Levin discovered a bug when fuzzing /dev/ecryptfs. The code behind
/dev/ecryptfs never considered the possibility that file descriptors may be
inherited or passed to other processes. Additionally, far too many BUG() calls
were used throughout the messaging code.
The eCryptfs messaging code was originally implemented using netlink. Several
years ago, it was converted to a miscdev driver but it retained some of the
concepts that were specific to netlink.
The first patch is a minimalist approach at solving the bug Sasha discovered.
The second patch removes some cruft. The last patch leverages the file-based
miscdev approach to simplify the implementation.
Tyler
---
fs/ecryptfs/ecryptfs_kernel.h | 22 ++-----
fs/ecryptfs/messaging.c | 136 +++++------------------------------------
fs/ecryptfs/miscdev.c | 91 ++++++++-------------------
3 files changed, 48 insertions(+), 201 deletions(-)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] eCryptfs: Gracefully refuse miscdev file ops on inherited/passed files
2012-06-13 0:05 ` [PATCH 0/3] eCryptfs: Fix and simplify messaging code Tyler Hicks
@ 2012-06-13 0:05 ` Tyler Hicks
2012-06-22 17:47 ` Sasha Levin
2012-06-13 0:05 ` [PATCH 2/3] eCryptfs: Remove unused messaging declarations and function Tyler Hicks
2012-06-13 0:05 ` [PATCH 3/3] eCryptfs: Make all miscdev functions use daemon ptr in file private_data Tyler Hicks
2 siblings, 1 reply; 8+ messages in thread
From: Tyler Hicks @ 2012-06-13 0:05 UTC (permalink / raw)
To: ecryptfs; +Cc: linux-kernel, Sasha Levin
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/994247
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Reported-by: Sasha Levin <levinsasha928@gmail.com>
Cc: Sasha Levin <levinsasha928@gmail.com>
---
fs/ecryptfs/miscdev.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/fs/ecryptfs/miscdev.c b/fs/ecryptfs/miscdev.c
index 3a06f40..2cd9c3f 100644
--- a/fs/ecryptfs/miscdev.c
+++ b/fs/ecryptfs/miscdev.c
@@ -49,7 +49,10 @@ ecryptfs_miscdev_poll(struct file *file, poll_table *pt)
mutex_lock(&ecryptfs_daemon_hash_mux);
/* TODO: Just use file->private_data? */
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_unlock(&ecryptfs_daemon_hash_mux);
if (daemon->flags & ECRYPTFS_DAEMON_ZOMBIE) {
@@ -122,6 +125,7 @@ ecryptfs_miscdev_open(struct inode *inode, struct file *file)
goto out_unlock_daemon;
}
daemon->flags |= ECRYPTFS_DAEMON_MISCDEV_OPEN;
+ file->private_data = daemon;
atomic_inc(&ecryptfs_num_miscdev_opens);
out_unlock_daemon:
mutex_unlock(&daemon->mux);
@@ -152,9 +156,9 @@ ecryptfs_miscdev_release(struct inode *inode, struct file *file)
mutex_lock(&ecryptfs_daemon_hash_mux);
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);
- BUG_ON(daemon->pid != task_pid(current));
BUG_ON(!(daemon->flags & ECRYPTFS_DAEMON_MISCDEV_OPEN));
daemon->flags &= ~ECRYPTFS_DAEMON_MISCDEV_OPEN;
atomic_dec(&ecryptfs_num_miscdev_opens);
@@ -269,8 +273,16 @@ ecryptfs_miscdev_read(struct file *file, char __user *buf, size_t count,
mutex_lock(&ecryptfs_daemon_hash_mux);
/* TODO: Just use file->private_data? */
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);
+ if (task_pid(current) != daemon->pid) {
+ mutex_unlock(&daemon->mux);
+ mutex_unlock(&ecryptfs_daemon_hash_mux);
+ return -EPERM;
+ }
if (daemon->flags & ECRYPTFS_DAEMON_ZOMBIE) {
rc = 0;
mutex_unlock(&ecryptfs_daemon_hash_mux);
@@ -307,9 +319,6 @@ check_list:
* message from the queue; try again */
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,
struct ecryptfs_msg_ctx, daemon_out_list);
BUG_ON(!msg_ctx);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] eCryptfs: Remove unused messaging declarations and function
2012-06-13 0:05 ` [PATCH 0/3] eCryptfs: Fix and simplify messaging code Tyler Hicks
2012-06-13 0:05 ` [PATCH 1/3] eCryptfs: Gracefully refuse miscdev file ops on inherited/passed files Tyler Hicks
@ 2012-06-13 0:05 ` Tyler Hicks
2012-06-13 0:05 ` [PATCH 3/3] eCryptfs: Make all miscdev functions use daemon ptr in file private_data Tyler Hicks
2 siblings, 0 replies; 8+ messages in thread
From: Tyler Hicks @ 2012-06-13 0:05 UTC (permalink / raw)
To: ecryptfs; +Cc: linux-kernel, Sasha Levin
These are no longer needed.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Cc: Sasha Levin <levinsasha928@gmail.com>
---
fs/ecryptfs/ecryptfs_kernel.h | 6 ------
fs/ecryptfs/messaging.c | 31 -------------------------------
2 files changed, 37 deletions(-)
diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
index 867b64c..01a1f85 100644
--- a/fs/ecryptfs/ecryptfs_kernel.h
+++ b/fs/ecryptfs/ecryptfs_kernel.h
@@ -385,8 +385,6 @@ struct ecryptfs_msg_ctx {
struct mutex mux;
};
-struct ecryptfs_daemon;
-
struct ecryptfs_daemon {
#define ECRYPTFS_DAEMON_IN_READ 0x00000001
#define ECRYPTFS_DAEMON_IN_POLL 0x00000002
@@ -621,10 +619,6 @@ int
ecryptfs_setxattr(struct dentry *dentry, const char *name, const void *value,
size_t size, int flags);
int ecryptfs_read_xattr_region(char *page_virt, struct inode *ecryptfs_inode);
-int ecryptfs_process_helo(uid_t euid, struct user_namespace *user_ns,
- struct pid *pid);
-int ecryptfs_process_quit(uid_t euid, struct user_namespace *user_ns,
- struct pid *pid);
int ecryptfs_process_response(struct ecryptfs_message *msg, uid_t euid,
struct user_namespace *user_ns, struct pid *pid,
u32 seq);
diff --git a/fs/ecryptfs/messaging.c b/fs/ecryptfs/messaging.c
index a750f95..c11911d 100644
--- a/fs/ecryptfs/messaging.c
+++ b/fs/ecryptfs/messaging.c
@@ -216,37 +216,6 @@ out:
}
/**
- * ecryptfs_process_quit
- * @euid: The user ID owner of the message
- * @user_ns: The namespace in which @euid applies
- * @pid: The process ID for the userspace program that sent the
- * message
- *
- * Deletes the corresponding daemon for the given euid and pid, if
- * it is the registered that is requesting the deletion. Returns zero
- * after deleting the desired daemon; non-zero otherwise.
- */
-int ecryptfs_process_quit(uid_t euid, struct user_namespace *user_ns,
- struct pid *pid)
-{
- struct ecryptfs_daemon *daemon;
- int rc;
-
- mutex_lock(&ecryptfs_daemon_hash_mux);
- rc = ecryptfs_find_daemon_by_euid(&daemon, euid, user_ns);
- if (rc || !daemon) {
- rc = -EINVAL;
- printk(KERN_ERR "Received request from user [%d] to "
- "unregister unrecognized daemon [0x%p]\n", euid, pid);
- goto out_unlock;
- }
- rc = ecryptfs_exorcise_daemon(daemon);
-out_unlock:
- mutex_unlock(&ecryptfs_daemon_hash_mux);
- return rc;
-}
-
-/**
* ecryptfs_process_reponse
* @msg: The ecryptfs message received; the caller should sanity check
* msg->data_len and free the memory
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] eCryptfs: Make all miscdev functions use daemon ptr in file private_data
2012-06-13 0:05 ` [PATCH 0/3] eCryptfs: Fix and simplify messaging code Tyler Hicks
2012-06-13 0:05 ` [PATCH 1/3] eCryptfs: Gracefully refuse miscdev file ops on inherited/passed files Tyler Hicks
2012-06-13 0:05 ` [PATCH 2/3] eCryptfs: Remove unused messaging declarations and function Tyler Hicks
@ 2012-06-13 0:05 ` Tyler Hicks
2 siblings, 0 replies; 8+ messages in thread
From: Tyler Hicks @ 2012-06-13 0:05 UTC (permalink / raw)
To: ecryptfs; +Cc: linux-kernel, Sasha Levin
Now that a pointer to a valid struct ecryptfs_daemon is stored in the
private_data of an opened /dev/ecryptfs file, the remaining miscdev
functions can utilize the pointer rather than looking up the
ecryptfs_daemon at the beginning of each operation.
The security model of /dev/ecryptfs is simplified a little bit with this
patch. Upon opening /dev/ecryptfs, a per-user ecryptfs_daemon is
registered. Another daemon cannot be registered for that user until the
last file reference is released. During the lifetime of the
ecryptfs_daemon, access checks are not performed on the /dev/ecryptfs
operations because it is assumed that the application securely handles
the opened file descriptor and does not unintentionally leak it to
processes that are not trusted.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Cc: Sasha Levin <levinsasha928@gmail.com>
---
fs/ecryptfs/ecryptfs_kernel.h | 16 ++-----
fs/ecryptfs/messaging.c | 105 +++++++----------------------------------
fs/ecryptfs/miscdev.c | 98 ++++++++++----------------------------
3 files changed, 47 insertions(+), 172 deletions(-)
diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
index 01a1f85..0deb4f2 100644
--- a/fs/ecryptfs/ecryptfs_kernel.h
+++ b/fs/ecryptfs/ecryptfs_kernel.h
@@ -392,10 +392,7 @@ struct ecryptfs_daemon {
#define ECRYPTFS_DAEMON_MISCDEV_OPEN 0x00000008
u32 flags;
u32 num_queued_msg_ctx;
- struct pid *pid;
- uid_t euid;
- struct user_namespace *user_ns;
- struct task_struct *task;
+ struct file *file;
struct mutex mux;
struct list_head msg_ctx_out_queue;
wait_queue_head_t wait;
@@ -619,9 +616,8 @@ int
ecryptfs_setxattr(struct dentry *dentry, const char *name, const void *value,
size_t size, int flags);
int ecryptfs_read_xattr_region(char *page_virt, struct inode *ecryptfs_inode);
-int ecryptfs_process_response(struct ecryptfs_message *msg, uid_t euid,
- struct user_namespace *user_ns, struct pid *pid,
- u32 seq);
+int ecryptfs_process_response(struct ecryptfs_daemon *daemon,
+ struct ecryptfs_message *msg, u32 seq);
int ecryptfs_send_message(char *data, int data_len,
struct ecryptfs_msg_ctx **msg_ctx);
int ecryptfs_wait_for_response(struct ecryptfs_msg_ctx *msg_ctx,
@@ -666,8 +662,7 @@ int ecryptfs_read_lower_page_segment(struct page *page_for_ecryptfs,
struct inode *ecryptfs_inode);
struct page *ecryptfs_get_locked_page(struct inode *inode, loff_t index);
int ecryptfs_exorcise_daemon(struct ecryptfs_daemon *daemon);
-int ecryptfs_find_daemon_by_euid(struct ecryptfs_daemon **daemon, uid_t euid,
- struct user_namespace *user_ns);
+int ecryptfs_find_daemon_by_euid(struct ecryptfs_daemon **daemon);
int ecryptfs_parse_packet_length(unsigned char *data, size_t *size,
size_t *length_size);
int ecryptfs_write_packet_length(char *dest, size_t size,
@@ -679,8 +674,7 @@ int ecryptfs_send_miscdev(char *data, size_t data_size,
u16 msg_flags, struct ecryptfs_daemon *daemon);
void ecryptfs_msg_ctx_alloc_to_free(struct ecryptfs_msg_ctx *msg_ctx);
int
-ecryptfs_spawn_daemon(struct ecryptfs_daemon **daemon, uid_t euid,
- struct user_namespace *user_ns, struct pid *pid);
+ecryptfs_spawn_daemon(struct ecryptfs_daemon **daemon, struct file *file);
int ecryptfs_init_kthread(void);
void ecryptfs_destroy_kthread(void);
int ecryptfs_privileged_open(struct file **lower_file,
diff --git a/fs/ecryptfs/messaging.c b/fs/ecryptfs/messaging.c
index c11911d..b29bb8b 100644
--- a/fs/ecryptfs/messaging.c
+++ b/fs/ecryptfs/messaging.c
@@ -32,8 +32,8 @@ static struct mutex ecryptfs_msg_ctx_lists_mux;
static struct hlist_head *ecryptfs_daemon_hash;
struct mutex ecryptfs_daemon_hash_mux;
static int ecryptfs_hash_bits;
-#define ecryptfs_uid_hash(uid) \
- hash_long((unsigned long)uid, ecryptfs_hash_bits)
+#define ecryptfs_current_euid_hash(uid) \
+ hash_long((unsigned long)current_euid(), ecryptfs_hash_bits)
static u32 ecryptfs_msg_counter;
static struct ecryptfs_msg_ctx *ecryptfs_msg_ctx_arr;
@@ -105,26 +105,24 @@ void ecryptfs_msg_ctx_alloc_to_free(struct ecryptfs_msg_ctx *msg_ctx)
/**
* ecryptfs_find_daemon_by_euid
- * @euid: The effective user id which maps to the desired daemon id
- * @user_ns: The namespace in which @euid applies
* @daemon: If return value is zero, points to the desired daemon pointer
*
* Must be called with ecryptfs_daemon_hash_mux held.
*
- * Search the hash list for the given user id.
+ * Search the hash list for the current effective user id.
*
* Returns zero if the user id exists in the list; non-zero otherwise.
*/
-int ecryptfs_find_daemon_by_euid(struct ecryptfs_daemon **daemon, uid_t euid,
- struct user_namespace *user_ns)
+int ecryptfs_find_daemon_by_euid(struct ecryptfs_daemon **daemon)
{
struct hlist_node *elem;
int rc;
hlist_for_each_entry(*daemon, elem,
- &ecryptfs_daemon_hash[ecryptfs_uid_hash(euid)],
- euid_chain) {
- if ((*daemon)->euid == euid && (*daemon)->user_ns == user_ns) {
+ &ecryptfs_daemon_hash[ecryptfs_current_euid_hash()],
+ euid_chain) {
+ if ((*daemon)->file->f_cred->euid == current_euid() &&
+ (*daemon)->file->f_cred->user_ns == current_user_ns()) {
rc = 0;
goto out;
}
@@ -137,9 +135,7 @@ out:
/**
* ecryptfs_spawn_daemon - Create and initialize a new daemon struct
* @daemon: Pointer to set to newly allocated daemon struct
- * @euid: Effective user id for the daemon
- * @user_ns: The namespace in which @euid applies
- * @pid: Process id for the daemon
+ * @file: File used when opening /dev/ecryptfs
*
* Must be called ceremoniously while in possession of
* ecryptfs_sacred_daemon_hash_mux
@@ -147,8 +143,7 @@ out:
* Returns zero on success; non-zero otherwise
*/
int
-ecryptfs_spawn_daemon(struct ecryptfs_daemon **daemon, uid_t euid,
- struct user_namespace *user_ns, struct pid *pid)
+ecryptfs_spawn_daemon(struct ecryptfs_daemon **daemon, struct file *file)
{
int rc = 0;
@@ -159,16 +154,13 @@ ecryptfs_spawn_daemon(struct ecryptfs_daemon **daemon, uid_t euid,
"GFP_KERNEL memory\n", __func__, sizeof(**daemon));
goto out;
}
- (*daemon)->euid = euid;
- (*daemon)->user_ns = get_user_ns(user_ns);
- (*daemon)->pid = get_pid(pid);
- (*daemon)->task = current;
+ (*daemon)->file = file;
mutex_init(&(*daemon)->mux);
INIT_LIST_HEAD(&(*daemon)->msg_ctx_out_queue);
init_waitqueue_head(&(*daemon)->wait);
(*daemon)->num_queued_msg_ctx = 0;
hlist_add_head(&(*daemon)->euid_chain,
- &ecryptfs_daemon_hash[ecryptfs_uid_hash(euid)]);
+ &ecryptfs_daemon_hash[ecryptfs_current_euid_hash()]);
out:
return rc;
}
@@ -188,9 +180,6 @@ int ecryptfs_exorcise_daemon(struct ecryptfs_daemon *daemon)
if ((daemon->flags & ECRYPTFS_DAEMON_IN_READ)
|| (daemon->flags & ECRYPTFS_DAEMON_IN_POLL)) {
rc = -EBUSY;
- printk(KERN_WARNING "%s: Attempt to destroy daemon with pid "
- "[0x%p], but it is in the midst of a read or a poll\n",
- __func__, daemon->pid);
mutex_unlock(&daemon->mux);
goto out;
}
@@ -203,12 +192,6 @@ int ecryptfs_exorcise_daemon(struct ecryptfs_daemon *daemon)
ecryptfs_msg_ctx_alloc_to_free(msg_ctx);
}
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);
kzfree(daemon);
out:
@@ -219,8 +202,6 @@ out:
* ecryptfs_process_reponse
* @msg: The ecryptfs message received; the caller should sanity check
* msg->data_len and free the memory
- * @pid: The process ID of the userspace application that sent the
- * message
* @seq: The sequence number of the message; must match the sequence
* number for the existing message context waiting for this
* response
@@ -239,16 +220,11 @@ out:
*
* Returns zero on success; non-zero otherwise
*/
-int ecryptfs_process_response(struct ecryptfs_message *msg, uid_t euid,
- struct user_namespace *user_ns, struct pid *pid,
- u32 seq)
+int ecryptfs_process_response(struct ecryptfs_daemon *daemon,
+ struct ecryptfs_message *msg, u32 seq)
{
- struct ecryptfs_daemon *uninitialized_var(daemon);
struct ecryptfs_msg_ctx *msg_ctx;
size_t msg_size;
- struct nsproxy *nsproxy;
- struct user_namespace *tsk_user_ns;
- uid_t ctx_euid;
int rc;
if (msg->index >= ecryptfs_message_buf_len) {
@@ -261,51 +237,6 @@ 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);
- 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;
- }
- tsk_user_ns = __task_cred(msg_ctx->task)->user_ns;
- ctx_euid = task_euid(msg_ctx->task);
- rc = ecryptfs_find_daemon_by_euid(&daemon, ctx_euid, tsk_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 [0x%p] but does "
- "not have a registered daemon\n", __func__,
- ctx_euid, pid);
- goto wake_up;
- }
- if (ctx_euid != euid) {
- rc = -EBADMSG;
- printk(KERN_WARNING "%s: Received message from user "
- "[%d]; expected message from user [%d]\n", __func__,
- euid, ctx_euid);
- goto unlock;
- }
- if (tsk_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",
- __func__, user_ns, tsk_user_ns);
- goto unlock;
- }
- if (daemon->pid != pid) {
- rc = -EBADMSG;
- printk(KERN_ERR "%s: User [%d] sent a message response "
- "from an unrecognized process [0x%p]\n",
- __func__, ctx_euid, pid);
- goto unlock;
- }
if (msg_ctx->state != ECRYPTFS_MSG_CTX_STATE_PENDING) {
rc = -EINVAL;
printk(KERN_WARNING "%s: Desired context element is not "
@@ -328,9 +259,8 @@ int ecryptfs_process_response(struct ecryptfs_message *msg, uid_t euid,
}
memcpy(msg_ctx->msg, msg, msg_size);
msg_ctx->state = ECRYPTFS_MSG_CTX_STATE_DONE;
- rc = 0;
-wake_up:
wake_up_process(msg_ctx->task);
+ rc = 0;
unlock:
mutex_unlock(&msg_ctx->mux);
out:
@@ -352,14 +282,11 @@ ecryptfs_send_message_locked(char *data, int data_len, u8 msg_type,
struct ecryptfs_msg_ctx **msg_ctx)
{
struct ecryptfs_daemon *daemon;
- uid_t euid = current_euid();
int rc;
- rc = ecryptfs_find_daemon_by_euid(&daemon, euid, current_user_ns());
+ rc = ecryptfs_find_daemon_by_euid(&daemon);
if (rc || !daemon) {
rc = -ENOTCONN;
- printk(KERN_ERR "%s: User [%d] does not have a daemon "
- "registered\n", __func__, euid);
goto out;
}
mutex_lock(&ecryptfs_msg_ctx_lists_mux);
diff --git a/fs/ecryptfs/miscdev.c b/fs/ecryptfs/miscdev.c
index 2cd9c3f..358156b 100644
--- a/fs/ecryptfs/miscdev.c
+++ b/fs/ecryptfs/miscdev.c
@@ -33,7 +33,7 @@ static atomic_t ecryptfs_num_miscdev_opens;
/**
* ecryptfs_miscdev_poll
- * @file: dev file (ignored)
+ * @file: dev file
* @pt: dev poll table (ignored)
*
* Returns the poll mask
@@ -41,20 +41,10 @@ static atomic_t ecryptfs_num_miscdev_opens;
static unsigned int
ecryptfs_miscdev_poll(struct file *file, poll_table *pt)
{
- struct ecryptfs_daemon *daemon;
+ struct ecryptfs_daemon *daemon = file->private_data;
unsigned int mask = 0;
- uid_t euid = current_euid();
- int rc;
- mutex_lock(&ecryptfs_daemon_hash_mux);
- /* TODO: Just use file->private_data? */
- rc = ecryptfs_find_daemon_by_euid(&daemon, euid, current_user_ns());
- if (rc || !daemon) {
- mutex_unlock(&ecryptfs_daemon_hash_mux);
- return -EINVAL;
- }
mutex_lock(&daemon->mux);
- mutex_unlock(&ecryptfs_daemon_hash_mux);
if (daemon->flags & ECRYPTFS_DAEMON_ZOMBIE) {
printk(KERN_WARNING "%s: Attempt to poll on zombified "
"daemon\n", __func__);
@@ -79,7 +69,7 @@ out_unlock_daemon:
/**
* ecryptfs_miscdev_open
* @inode: inode of miscdev handle (ignored)
- * @file: file for miscdev handle (ignored)
+ * @file: file for miscdev handle
*
* Returns zero on success; non-zero otherwise
*/
@@ -87,7 +77,6 @@ static int
ecryptfs_miscdev_open(struct inode *inode, struct file *file)
{
struct ecryptfs_daemon *daemon = NULL;
- uid_t euid = current_euid();
int rc;
mutex_lock(&ecryptfs_daemon_hash_mux);
@@ -98,30 +87,20 @@ ecryptfs_miscdev_open(struct inode *inode, struct file *file)
"count; rc = [%d]\n", __func__, rc);
goto out_unlock_daemon_list;
}
- rc = ecryptfs_find_daemon_by_euid(&daemon, euid, current_user_ns());
- if (rc || !daemon) {
- rc = ecryptfs_spawn_daemon(&daemon, euid, current_user_ns(),
- task_pid(current));
- if (rc) {
- printk(KERN_ERR "%s: Error attempting to spawn daemon; "
- "rc = [%d]\n", __func__, rc);
- goto out_module_put_unlock_daemon_list;
- }
- }
- mutex_lock(&daemon->mux);
- if (daemon->pid != task_pid(current)) {
+ rc = ecryptfs_find_daemon_by_euid(&daemon);
+ if (!rc) {
rc = -EINVAL;
- printk(KERN_ERR "%s: pid [0x%p] has registered with euid [%d], "
- "but pid [0x%p] has attempted to open the handle "
- "instead\n", __func__, daemon->pid, daemon->euid,
- task_pid(current));
- goto out_unlock_daemon;
+ goto out_unlock_daemon_list;
+ }
+ rc = ecryptfs_spawn_daemon(&daemon, file);
+ if (rc) {
+ printk(KERN_ERR "%s: Error attempting to spawn daemon; "
+ "rc = [%d]\n", __func__, rc);
+ goto out_module_put_unlock_daemon_list;
}
+ mutex_lock(&daemon->mux);
if (daemon->flags & ECRYPTFS_DAEMON_MISCDEV_OPEN) {
rc = -EBUSY;
- printk(KERN_ERR "%s: Miscellaneous device handle may only be "
- "opened once per daemon; pid [0x%p] already has this "
- "handle open\n", __func__, daemon->pid);
goto out_unlock_daemon;
}
daemon->flags |= ECRYPTFS_DAEMON_MISCDEV_OPEN;
@@ -140,7 +119,7 @@ out_unlock_daemon_list:
/**
* ecryptfs_miscdev_release
* @inode: inode of fs/ecryptfs/euid handle (ignored)
- * @file: file for fs/ecryptfs/euid handle (ignored)
+ * @file: file for fs/ecryptfs/euid handle
*
* This keeps the daemon registered until the daemon sends another
* ioctl to fs/ecryptfs/ctl or until the kernel module unregisters.
@@ -150,20 +129,18 @@ out_unlock_daemon_list:
static int
ecryptfs_miscdev_release(struct inode *inode, struct file *file)
{
- struct ecryptfs_daemon *daemon = NULL;
- uid_t euid = current_euid();
+ struct ecryptfs_daemon *daemon = file->private_data;
int rc;
- mutex_lock(&ecryptfs_daemon_hash_mux);
- rc = ecryptfs_find_daemon_by_euid(&daemon, euid, current_user_ns());
- if (rc || !daemon)
- daemon = file->private_data;
mutex_lock(&daemon->mux);
BUG_ON(!(daemon->flags & ECRYPTFS_DAEMON_MISCDEV_OPEN));
daemon->flags &= ~ECRYPTFS_DAEMON_MISCDEV_OPEN;
atomic_dec(&ecryptfs_num_miscdev_opens);
mutex_unlock(&daemon->mux);
+
+ mutex_lock(&ecryptfs_daemon_hash_mux);
rc = ecryptfs_exorcise_daemon(daemon);
+ mutex_unlock(&ecryptfs_daemon_hash_mux);
if (rc) {
printk(KERN_CRIT "%s: Fatal error whilst attempting to "
"shut down daemon; rc = [%d]. Please report this "
@@ -171,7 +148,6 @@ ecryptfs_miscdev_release(struct inode *inode, struct file *file)
BUG();
}
module_put(THIS_MODULE);
- mutex_unlock(&ecryptfs_daemon_hash_mux);
return rc;
}
@@ -247,7 +223,7 @@ out_unlock:
/**
* ecryptfs_miscdev_read - format and send message from queue
- * @file: fs/ecryptfs/euid miscdevfs handle (ignored)
+ * @file: miscdevfs handle
* @buf: User buffer into which to copy the next message on the daemon queue
* @count: Amount of space available in @buf
* @ppos: Offset in file (ignored)
@@ -261,43 +237,27 @@ static ssize_t
ecryptfs_miscdev_read(struct file *file, char __user *buf, size_t count,
loff_t *ppos)
{
- struct ecryptfs_daemon *daemon;
+ struct ecryptfs_daemon *daemon = file->private_data;
struct ecryptfs_msg_ctx *msg_ctx;
size_t packet_length_size;
char packet_length[ECRYPTFS_MAX_PKT_LEN_SIZE];
size_t i;
size_t total_length;
- uid_t euid = current_euid();
int rc;
- mutex_lock(&ecryptfs_daemon_hash_mux);
- /* TODO: Just use file->private_data? */
- rc = ecryptfs_find_daemon_by_euid(&daemon, euid, current_user_ns());
- if (rc || !daemon) {
- mutex_unlock(&ecryptfs_daemon_hash_mux);
- return -EINVAL;
- }
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) {
rc = 0;
- mutex_unlock(&ecryptfs_daemon_hash_mux);
printk(KERN_WARNING "%s: Attempt to read from zombified "
"daemon\n", __func__);
goto out_unlock_daemon;
}
if (daemon->flags & ECRYPTFS_DAEMON_IN_READ) {
rc = 0;
- mutex_unlock(&ecryptfs_daemon_hash_mux);
goto out_unlock_daemon;
}
/* This daemon will not go away so long as this flag is set */
daemon->flags |= ECRYPTFS_DAEMON_IN_READ;
- mutex_unlock(&ecryptfs_daemon_hash_mux);
check_list:
if (list_empty(&daemon->msg_ctx_out_queue)) {
mutex_unlock(&daemon->mux);
@@ -381,16 +341,12 @@ out_unlock_daemon:
* ecryptfs_miscdev_response - miscdevess response to message previously sent to daemon
* @data: Bytes comprising struct ecryptfs_message
* @data_size: sizeof(struct ecryptfs_message) + data len
- * @euid: Effective user id of miscdevess sending the miscdev response
- * @user_ns: The namespace in which @euid applies
- * @pid: Miscdevess id of miscdevess sending the miscdev response
* @seq: Sequence number for miscdev response packet
*
* Returns zero on success; non-zero otherwise
*/
-static int ecryptfs_miscdev_response(char *data, size_t data_size,
- uid_t euid, struct user_namespace *user_ns,
- struct pid *pid, u32 seq)
+static int ecryptfs_miscdev_response(struct ecryptfs_daemon *daemon, char *data,
+ size_t data_size, u32 seq)
{
struct ecryptfs_message *msg = (struct ecryptfs_message *)data;
int rc;
@@ -402,7 +358,7 @@ static int ecryptfs_miscdev_response(char *data, size_t data_size,
rc = -EINVAL;
goto out;
}
- rc = ecryptfs_process_response(msg, euid, user_ns, pid, seq);
+ rc = ecryptfs_process_response(daemon, msg, seq);
if (rc)
printk(KERN_ERR
"Error processing response message; rc = [%d]\n", rc);
@@ -412,7 +368,7 @@ out:
/**
* ecryptfs_miscdev_write - handle write to daemon miscdev handle
- * @file: File for misc dev handle (ignored)
+ * @file: File for misc dev handle
* @buf: Buffer containing user data
* @count: Amount of data in @buf
* @ppos: Pointer to offset in file (ignored)
@@ -427,7 +383,6 @@ ecryptfs_miscdev_write(struct file *file, const char __user *buf,
u32 seq;
size_t packet_size, packet_size_length;
char *data;
- uid_t euid = current_euid();
unsigned char packet_size_peek[ECRYPTFS_MAX_PKT_LEN_SIZE];
ssize_t rc;
@@ -487,10 +442,9 @@ memdup:
}
memcpy(&counter_nbo, &data[PKT_CTR_OFFSET], PKT_CTR_SIZE);
seq = be32_to_cpu(counter_nbo);
- rc = ecryptfs_miscdev_response(
+ rc = ecryptfs_miscdev_response(file->private_data,
&data[PKT_LEN_OFFSET + packet_size_length],
- packet_size, euid, current_user_ns(),
- task_pid(current), seq);
+ packet_size, seq);
if (rc) {
printk(KERN_WARNING "%s: Failed to deliver miscdev "
"response to requesting operation; rc = [%zd]\n",
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] eCryptfs: Gracefully refuse miscdev file ops on inherited/passed files
2012-06-13 0:05 ` [PATCH 1/3] eCryptfs: Gracefully refuse miscdev file ops on inherited/passed files Tyler Hicks
@ 2012-06-22 17:47 ` Sasha Levin
2012-06-22 17:58 ` Tyler Hicks
0 siblings, 1 reply; 8+ messages in thread
From: Sasha Levin @ 2012-06-22 17:47 UTC (permalink / raw)
To: Tyler Hicks; +Cc: ecryptfs, linux-kernel
On Wed, Jun 13, 2012 at 2:05 AM, Tyler Hicks <tyhicks@canonical.com> wrote:
> 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/994247
>
> Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
> Reported-by: Sasha Levin <levinsasha928@gmail.com>
> Cc: Sasha Levin <levinsasha928@gmail.com>
> ---
I've been running it a while now and haven't seen the problem I've
reported reproducing.
Is it possible to merge this fix into 3.5?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] eCryptfs: Gracefully refuse miscdev file ops on inherited/passed files
2012-06-22 17:47 ` Sasha Levin
@ 2012-06-22 17:58 ` Tyler Hicks
0 siblings, 0 replies; 8+ messages in thread
From: Tyler Hicks @ 2012-06-22 17:58 UTC (permalink / raw)
To: Sasha Levin; +Cc: ecryptfs, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1473 bytes --]
On 2012-06-22 19:47:35, Sasha Levin wrote:
> On Wed, Jun 13, 2012 at 2:05 AM, Tyler Hicks <tyhicks@canonical.com> wrote:
> > 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/994247
> >
> > Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
> > Reported-by: Sasha Levin <levinsasha928@gmail.com>
> > Cc: Sasha Levin <levinsasha928@gmail.com>
> > ---
>
> I've been running it a while now and haven't seen the problem I've
> reported reproducing.
Thanks for the testing!
>
> Is it possible to merge this fix into 3.5?
That's my plan. I'll target this bug fix and the 2nd patch that removes
unused code for 3.5. I'll wait for 3.6 to merge the third patch.
Tyler
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-06-22 17:58 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-26 19:39 ecryptfs: kernel BUG at fs/ecryptfs/miscdev.c:52 Sasha Levin
2012-06-07 0:41 ` Sasha Levin
2012-06-13 0:05 ` [PATCH 0/3] eCryptfs: Fix and simplify messaging code Tyler Hicks
2012-06-13 0:05 ` [PATCH 1/3] eCryptfs: Gracefully refuse miscdev file ops on inherited/passed files Tyler Hicks
2012-06-22 17:47 ` Sasha Levin
2012-06-22 17:58 ` Tyler Hicks
2012-06-13 0:05 ` [PATCH 2/3] eCryptfs: Remove unused messaging declarations and function Tyler Hicks
2012-06-13 0:05 ` [PATCH 3/3] eCryptfs: Make all miscdev functions use daemon ptr in file private_data Tyler Hicks
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).