linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC/PATCH] revoke/frevoke system calls
@ 2006-07-20 12:07 Pekka J Enberg
  2006-07-20 20:02 ` Kari Hurtta
  2006-07-22  0:19 ` Andrew Morton
  0 siblings, 2 replies; 13+ messages in thread
From: Pekka J Enberg @ 2006-07-20 12:07 UTC (permalink / raw)
  To: alan, tytso; +Cc: linux-kernel, linux-fsdevel

From: Pekka Enberg <penberg@cs.helsinki.fi>

This patch implements the revoke(2) and frevoke(2) system calls for all
types of files. We revoke files in two passes: first we scan all open 
files that refer to the inode and substitute the struct file pointer in fd 
table with NULL causing all subsequent operations on that fd to fail. 
After we have done that to all file descriptors, we close the files and 
take down mmaps.

Note that now we need to unconditionally do fput/fget in sys_write and
sys_read because they race with do_revoke.

Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---

 arch/i386/kernel/syscall_table.S |    2 
 fs/Makefile                      |    2 
 fs/read_write.c                  |   10 -
 fs/revoke.c                      |  207 +++++++++++++++++++++++++++++++++++++++
 include/asm-i386/unistd.h        |    4 
 include/linux/syscalls.h         |    3 
 6 files changed, 220 insertions(+), 8 deletions(-)

Index: 2.6/arch/i386/kernel/syscall_table.S
===================================================================
--- 2.6.orig/arch/i386/kernel/syscall_table.S
+++ 2.6/arch/i386/kernel/syscall_table.S
@@ -317,3 +317,5 @@ ENTRY(sys_call_table)
 	.long sys_tee			/* 315 */
 	.long sys_vmsplice
 	.long sys_move_pages
+	.long sys_revoke
+	.long sys_frevoke
Index: 2.6/fs/Makefile
===================================================================
--- 2.6.orig/fs/Makefile
+++ 2.6/fs/Makefile
@@ -10,7 +10,7 @@ obj-y :=	open.o read_write.o file_table.
 		ioctl.o readdir.o select.o fifo.o locks.o dcache.o inode.o \
 		attr.o bad_inode.o file.o filesystems.o namespace.o aio.o \
 		seq_file.o xattr.o libfs.o fs-writeback.o mpage.o direct-io.o \
-		ioprio.o pnode.o drop_caches.o splice.o sync.o
+		ioprio.o pnode.o drop_caches.o splice.o sync.o revoke.o
 
 obj-$(CONFIG_INOTIFY)		+= inotify.o
 obj-$(CONFIG_INOTIFY_USER)	+= inotify_user.o
Index: 2.6/fs/read_write.c
===================================================================
--- 2.6.orig/fs/read_write.c
+++ 2.6/fs/read_write.c
@@ -343,14 +343,13 @@ asmlinkage ssize_t sys_read(unsigned int
 {
 	struct file *file;
 	ssize_t ret = -EBADF;
-	int fput_needed;
 
-	file = fget_light(fd, &fput_needed);
+	file = fget(fd);
 	if (file) {
 		loff_t pos = file_pos_read(file);
 		ret = vfs_read(file, buf, count, &pos);
 		file_pos_write(file, pos);
-		fput_light(file, fput_needed);
+		fput(file);
 	}
 
 	return ret;
@@ -361,14 +360,13 @@ asmlinkage ssize_t sys_write(unsigned in
 {
 	struct file *file;
 	ssize_t ret = -EBADF;
-	int fput_needed;
 
-	file = fget_light(fd, &fput_needed);
+	file = fget(fd);
 	if (file) {
 		loff_t pos = file_pos_read(file);
 		ret = vfs_write(file, buf, count, &pos);
 		file_pos_write(file, pos);
-		fput_light(file, fput_needed);
+		fput(file);
 	}
 
 	return ret;
Index: 2.6/fs/revoke.c
===================================================================
--- /dev/null
+++ 2.6/fs/revoke.c
@@ -0,0 +1,207 @@
+/*
+ * fs/revoke.c - Invalidate all current open file descriptors of an inode.
+ *
+ * Copyright (C) 2006 Pekka Enberg
+ *
+ * This file is released under the GPLv2.
+ */
+#include <linux/file.h>
+#include <linux/fs.h>
+#include <linux/namei.h>
+#include <linux/mm.h>
+#include <linux/sched.h>
+
+/*
+ * Auxiliary struct for keeping track of revoked files.
+ */
+struct revoked_file {
+	struct file *file;
+	struct task_struct *owner;
+	struct list_head list_node;
+};
+
+/*
+ * 	LOCKING: task_lock(owner)
+ */
+static int revoke_files(struct task_struct *owner, struct inode *inode,
+			struct file *exclude, struct list_head *to_close)
+{
+	int err = 0;
+	struct files_struct *files;
+	struct fdtable *fdt;
+	unsigned int fd;
+
+	files = owner->files;
+	spin_lock(&files->file_lock);
+
+	fdt = files_fdtable(files);
+	for (fd = 0; fd < fdt->max_fds; fd++) {
+		struct file *file;
+		struct revoked_file *revoked;
+
+		file = fcheck_files(files, fd);
+		if (!file)
+			continue;
+
+		if (file == exclude)
+			continue;
+
+		if (file->f_dentry->d_inode != inode)
+			continue;
+
+		/*
+		 * Leak the fd so it is not reused. After this point, we don't
+		 * need to worry about racing with sys_close or sys_dup.
+		 */
+		rcu_assign_pointer(fdt->fd[fd], NULL);
+		FD_CLR(fd, fdt->close_on_exec);
+
+		revoked = kmalloc(sizeof(*revoked), GFP_KERNEL);
+		if (!revoked) {
+			err = -ENOMEM;
+			goto out;
+		}
+
+		INIT_LIST_HEAD(&revoked->list_node);
+		revoked->file  = file;
+		revoked->owner = owner;
+
+		/*
+		 * Hold on to task until we can take down the file and its
+		 * mmap.
+		 */
+		get_task_struct(owner);
+
+		list_add(&revoked->list_node, to_close);
+	}
+  out:
+	spin_unlock(&files->file_lock);
+	return err;
+}
+
+static struct page *revoked_nopage(struct vm_area_struct *area,
+				   unsigned long address, int *type)
+{
+	return NULL;
+}
+
+static struct vm_operations_struct revoked_vm_ops = {
+        .nopage         = revoked_nopage,
+};
+
+static int revoke_mapping(struct address_space *mapping)
+{
+	struct vm_area_struct *vma;
+	struct prio_tree_iter iter;
+
+	spin_lock(&mapping->i_mmap_lock);
+	/* make ->nopage fail for all mmaps of the mapping */
+	vma_prio_tree_foreach(vma, &iter, &mapping->i_mmap, 0, ULONG_MAX)
+		vma->vm_ops = &revoked_vm_ops;
+	list_for_each_entry(vma, &mapping->i_mmap_nonlinear, shared.vm_set.list)
+		vma->vm_ops = &revoked_vm_ops;
+	spin_unlock(&mapping->i_mmap_lock);
+
+	/* FIXME: If we fail to invalidate some pages, no one will take them
+	   down but subsequent revoke operations succeed... */
+	return invalidate_inode_pages2(mapping);
+}
+
+static int close_files(struct list_head *to_close)
+{
+	int ret = 0;
+	struct revoked_file *this, *next;
+
+	list_for_each_entry_safe(this, next, to_close, list_node) {
+		struct inode *inode;
+		struct task_struct *task;
+		int err;
+
+		inode = this->file->f_dentry->d_inode;
+
+		task = this->owner;
+		task_lock(task);
+		if (task->files) {
+			err = filp_close(this->file, task->files);
+			if (err)
+				ret = err;
+		}
+		task_unlock(task);
+		put_task_struct(task);
+
+		err = revoke_mapping(inode->i_mapping);
+		if (err)
+			ret = err;
+
+		list_del(&this->list_node);
+		kfree(this);
+	}
+	return ret;
+}
+
+static int do_revoke(struct inode *inode, struct file *exclude)
+{
+	int err, ret = 0;
+	struct task_struct *g, *p;
+	struct list_head to_close = LIST_HEAD_INIT(to_close);
+
+	if (current->fsuid != inode->i_uid && !capable(CAP_FOWNER)) {
+		ret = -EPERM;
+		goto out;
+	}
+
+	/*
+	 * First revoke the file descriptors. After we are done, all new
+	 * operations on the descriptors will fail.
+	 */
+	read_lock(&tasklist_lock);
+	do_each_thread(g, p) {
+		task_lock(p);
+
+		if (p->files) {
+			ret = revoke_files(p, inode, exclude, &to_close);
+			if (ret) {
+				task_unlock(p);
+				goto out_unlock_tasklist;
+			}
+		}
+		task_unlock(p);
+	} while_each_thread(g, p);
+
+  out_unlock_tasklist:
+	read_unlock(&tasklist_lock);
+
+	/*
+	 * Now, close the files and take down mmaps.
+	 */
+	err = close_files(&to_close);
+	if (err)
+		ret = err;
+  out:
+	return ret;
+}
+
+asmlinkage int sys_revoke(const char __user *filename)
+{
+	int err;
+	struct nameidata nd;
+
+	err = __user_walk(filename, 0, &nd);
+	if (!err) {
+		err = do_revoke(nd.dentry->d_inode, NULL);
+		path_release(&nd);
+	}
+	return err;
+}
+
+asmlinkage int sys_frevoke(unsigned int fd)
+{
+	struct file *file = fget(fd);
+	int err = -EBADF;
+
+	if (file) {
+		err = do_revoke(file->f_dentry->d_inode, file);
+		fput(file);
+	}
+	return err;
+}
Index: 2.6/include/asm-i386/unistd.h
===================================================================
--- 2.6.orig/include/asm-i386/unistd.h
+++ 2.6/include/asm-i386/unistd.h
@@ -323,10 +323,12 @@
 #define __NR_tee		315
 #define __NR_vmsplice		316
 #define __NR_move_pages		317
+#define __NR_revoke		318
+#define __NR_frevoke		319
 
 #ifdef __KERNEL__
 
-#define NR_syscalls 318
+#define NR_syscalls 320
 
 /*
  * user-visible error numbers are in the range -1 - -128: see
Index: 2.6/include/linux/syscalls.h
===================================================================
--- 2.6.orig/include/linux/syscalls.h
+++ 2.6/include/linux/syscalls.h
@@ -597,4 +597,7 @@ asmlinkage long sys_get_robust_list(int 
 asmlinkage long sys_set_robust_list(struct robust_list_head __user *head,
 				    size_t len);
 
+asmlinkage int sys_revoke(const char __user *filename);
+asmlinkage int sys_frevoke(unsigned int fd);
+
 #endif

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC/PATCH] revoke/frevoke system calls
  2006-07-20 12:07 [RFC/PATCH] revoke/frevoke system calls Pekka J Enberg
@ 2006-07-20 20:02 ` Kari Hurtta
  2006-07-20 21:09   ` Björn Steinbrink
  2006-07-22  0:19 ` Andrew Morton
  1 sibling, 1 reply; 13+ messages in thread
From: Kari Hurtta @ 2006-07-20 20:02 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-fsdevel

Pekka J Enberg <penberg@cs.Helsinki.FI> writes in
gmane.linux.file-systems,gmane.linux.kernel:

> From: Pekka Enberg <penberg@cs.helsinki.fi>
> 
> This patch implements the revoke(2) and frevoke(2) system calls for all
> types of files. We revoke files in two passes: first we scan all open 
> files that refer to the inode and substitute the struct file pointer in fd 
> table with NULL causing all subsequent operations on that fd to fail. 
> After we have done that to all file descriptors, we close the files and 
> take down mmaps.
> 
> Note that now we need to unconditionally do fput/fget in sys_write and
> sys_read because they race with do_revoke.
> 
> Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>

What permissions is needed revoke access of other users open
files ?

> +asmlinkage int sys_revoke(const char __user *filename)
> +{
> +	int err;
> +	struct nameidata nd;
> +
> +	err = __user_walk(filename, 0, &nd);
> +	if (!err) {
> +		err = do_revoke(nd.dentry->d_inode, NULL);
> +		path_release(&nd);
> +	}
> +	return err;
> +}
> +
> +asmlinkage int sys_frevoke(unsigned int fd)
> +{
> +	struct file *file = fget(fd);
> +	int err = -EBADF;
> +
> +	if (file) {
> +		err = do_revoke(file->f_dentry->d_inode, file);
> +		fput(file);
> +	}
> +	return err;
> +}

Is that requiring only that user is able to refer file ?


BSD manual page for revoke(2) seems say:

    Access to a file may be revoked only by its owner or the super user.

/ Kari Hurtta

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC/PATCH] revoke/frevoke system calls
  2006-07-20 20:02 ` Kari Hurtta
@ 2006-07-20 21:09   ` Björn Steinbrink
  2006-07-21 22:16     ` Edgar Toernig
  0 siblings, 1 reply; 13+ messages in thread
From: Björn Steinbrink @ 2006-07-20 21:09 UTC (permalink / raw)
  To: Kari Hurtta; +Cc: linux-kernel, linux-fsdevel

On 2006.07.20 23:02:53 +0300, Kari Hurtta wrote:
> Pekka J Enberg <penberg@cs.Helsinki.FI> writes in
> gmane.linux.file-systems,gmane.linux.kernel:
> 
> > From: Pekka Enberg <penberg@cs.helsinki.fi>
> > 
> > This patch implements the revoke(2) and frevoke(2) system calls for all
> > types of files. We revoke files in two passes: first we scan all open 
> > files that refer to the inode and substitute the struct file pointer in fd 
> > table with NULL causing all subsequent operations on that fd to fail. 
> > After we have done that to all file descriptors, we close the files and 
> > take down mmaps.
> > 
> > Note that now we need to unconditionally do fput/fget in sys_write and
> > sys_read because they race with do_revoke.
> > 
> > Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
> 
> What permissions is needed revoke access of other users open
> files ?
> 
> > +asmlinkage int sys_revoke(const char __user *filename)
> > +{
> > +	int err;
> > +	struct nameidata nd;
> > +
> > +	err = __user_walk(filename, 0, &nd);
> > +	if (!err) {
> > +		err = do_revoke(nd.dentry->d_inode, NULL);
> > +		path_release(&nd);
> > +	}
> > +	return err;
> > +}
> > +
> > +asmlinkage int sys_frevoke(unsigned int fd)
> > +{
> > +	struct file *file = fget(fd);
> > +	int err = -EBADF;
> > +
> > +	if (file) {
> > +		err = do_revoke(file->f_dentry->d_inode, file);
> > +		fput(file);
> > +	}
> > +	return err;
> > +}
> 
> Is that requiring only that user is able to refer file ?
> 
> 
> BSD manual page for revoke(2) seems say:
> 
>     Access to a file may be revoked only by its owner or the super user.

In do_revoke() there is:

+	if (current->fsuid != inode->i_uid && !capable(CAP_FOWNER)) {
+		ret = -EPERM;
+		goto out;

That pretty much matches what the BSD manpage says.

Björn
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC/PATCH] revoke/frevoke system calls
       [not found] <6AFvY-7ZK-15@gated-at.bofh.it>
@ 2006-07-20 21:26 ` Bodo Eggert
  2006-07-20 23:01   ` roucaries bastien
  0 siblings, 1 reply; 13+ messages in thread
From: Bodo Eggert @ 2006-07-20 21:26 UTC (permalink / raw)
  To: Pekka J Enberg, alan, tytso, linux-kernel, linux-fsdevel

Pekka J Enberg <penberg@cs.Helsinki.FI> wrote:
> From: Pekka Enberg <penberg@cs.helsinki.fi>

> This patch implements the revoke(2) and frevoke(2) system calls for all
> types of files. We revoke files in two passes: first we scan all open
> files that refer to the inode and substitute the struct file pointer in fd
> table with NULL causing all subsequent operations on that fd to fail.
> After we have done that to all file descriptors, we close the files and
> take down mmaps.

RFC2: Make umount -f work on local fs using this feature.
-- 
Ich danke GMX dafür, die Verwendung meiner Adressen mittels per SPF
verbreiteten Lügen zu sabotieren.

http://david.woodhou.se/why-not-spf.html
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC/PATCH] revoke/frevoke system calls
  2006-07-20 21:26 ` Bodo Eggert
@ 2006-07-20 23:01   ` roucaries bastien
  0 siblings, 0 replies; 13+ messages in thread
From: roucaries bastien @ 2006-07-20 23:01 UTC (permalink / raw)
  To: 7eggert; +Cc: Pekka J Enberg, alan, tytso, linux-kernel, linux-fsdevel

On 7/20/06, Bodo Eggert <7eggert@elstempel.de> wrote:
> Pekka J Enberg <penberg@cs.Helsinki.FI> wrote:
> > From: Pekka Enberg <penberg@cs.helsinki.fi>
>
> > This patch implements the revoke(2) and frevoke(2) system calls for all
> > types of files. We revoke files in two passes: first we scan all open
> > files that refer to the inode and substitute the struct file pointer in fd
> > table with NULL causing all subsequent operations on that fd to fail.
> > After we have done that to all file descriptors, we close the files and
> > take down mmaps.
>
> RFC2: Make umount -f work on local fs using this feature.
RFC3: use preliminary work about umount -f in order to get revoke for free.
See for instance
http://developer.osdl.org/dev/fumount/kernel2/patches/2.6.12/1/forced-unmount-2.6.12-1.patch

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC/PATCH] revoke/frevoke system calls
  2006-07-20 21:09   ` Björn Steinbrink
@ 2006-07-21 22:16     ` Edgar Toernig
  0 siblings, 0 replies; 13+ messages in thread
From: Edgar Toernig @ 2006-07-21 22:16 UTC (permalink / raw)
  To: Björn Steinbrink; +Cc: Kari Hurtta, linux-kernel, linux-fsdevel

Björn Steinbrink wrote:
>
> On 2006.07.20 23:02:53 +0300, Kari Hurtta wrote:
> > Pekka J Enberg <penberg@cs.Helsinki.FI> writes in
> > gmane.linux.file-systems,gmane.linux.kernel:
> > 
> > > From: Pekka Enberg <penberg@cs.helsinki.fi>
> > > 
> > > [...]
> > > After we have done that to all file descriptors, we close the files and 
> > > take down mmaps.
> > 
> > What permissions is needed revoke access of other users open
> > files ?
> > [...]
> > BSD manual page for revoke(2) seems say:
> > 
> >     Access to a file may be revoked only by its owner or the super user.
> 
> In do_revoke() there is:
> 
> +	if (current->fsuid != inode->i_uid && !capable(CAP_FOWNER)) {
> +		ret = -EPERM;
> +		goto out;
> 
> That pretty much matches what the BSD manpage says.

Urgs, so any user may remove mappings from another process and
let it crash?

Reading revoke man pages, it seems the system call was meant for
tty devices.  While I can see the generic case  to be useful for
forced unmount, it seems pretty dangerous to allow any user to
revoke file access and tear down mappings.

Who needs the generic case anyway?  IMO that's too much rope.
The effect on affected processes is nearly unpredictable.
IMHO, fuser is "good enough(tm)".

Btw, grep revoke POSIX.txt -> nil.

Ciao, ET.
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC/PATCH] revoke/frevoke system calls
  2006-07-20 12:07 [RFC/PATCH] revoke/frevoke system calls Pekka J Enberg
  2006-07-20 20:02 ` Kari Hurtta
@ 2006-07-22  0:19 ` Andrew Morton
  2006-07-22  6:22   ` Pekka J Enberg
  1 sibling, 1 reply; 13+ messages in thread
From: Andrew Morton @ 2006-07-22  0:19 UTC (permalink / raw)
  To: Pekka J Enberg; +Cc: alan, tytso, linux-kernel, linux-fsdevel

On Thu, 20 Jul 2006 15:07:30 +0300 (EEST)
Pekka J Enberg <penberg@cs.Helsinki.FI> wrote:

> From: Pekka Enberg <penberg@cs.helsinki.fi>
> 
> This patch implements the revoke(2) and frevoke(2) system calls for all
> types of files.
>
> ...
>
> -	file = fget_light(fd, &fput_needed);
> +	file = fget(fd);

This is sad.

> +static int revoke_files(struct task_struct *owner, struct inode *inode,
> +			struct file *exclude, struct list_head *to_close)
> +{
> ...
> +	spin_lock(&files->file_lock);
> ...
> +		revoked = kmalloc(sizeof(*revoked), GFP_KERNEL);

This is bad.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC/PATCH] revoke/frevoke system calls
  2006-07-22  0:19 ` Andrew Morton
@ 2006-07-22  6:22   ` Pekka J Enberg
  2006-07-22  6:59     ` Andrew Morton
  0 siblings, 1 reply; 13+ messages in thread
From: Pekka J Enberg @ 2006-07-22  6:22 UTC (permalink / raw)
  To: Andrew Morton; +Cc: alan, tytso, linux-kernel, linux-fsdevel

On Thu, 20 Jul 2006 15:07:30 +0300 (EEST)
Pekka J Enberg <penberg@cs.Helsinki.FI> wrote:
> > This patch implements the revoke(2) and frevoke(2) system calls for all
> > types of files.
> >
> > ...
> >
> > -	file = fget_light(fd, &fput_needed);
> > +	file = fget(fd);
 
On Fri, 21 Jul 2006, Andrew Morton wrote:
> This is sad.

There are alternatives, playing games with ->f_op, creating fake struct 
file, and doing IS_REVOKED if-else in the paths, but I think this is by 
far the simplest way to do it. So in the Andrew scale of sads, how 
sad is it, exactly?-)
 
On Thu, 20 Jul 2006 15:07:30 +0300 (EEST)
Pekka J Enberg <penberg@cs.Helsinki.FI> wrote:
> > +static int revoke_files(struct task_struct *owner, struct inode *inode,
> > +			struct file *exclude, struct list_head *to_close)
> > +{
> > ...
> > +	spin_lock(&files->file_lock);
> > ...
> > +		revoked = kmalloc(sizeof(*revoked), GFP_KERNEL);
 
On Fri, 21 Jul 2006, Andrew Morton wrote:
> This is bad.

Indeed, I'll come up with a better one as soon as I sort out the mmap 
takedown issues.

Thanks Andrew.

					Pekka

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC/PATCH] revoke/frevoke system calls
  2006-07-22  6:22   ` Pekka J Enberg
@ 2006-07-22  6:59     ` Andrew Morton
  2006-07-22  7:41       ` Pekka Enberg
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Morton @ 2006-07-22  6:59 UTC (permalink / raw)
  To: Pekka J Enberg; +Cc: alan, tytso, linux-kernel, linux-fsdevel

On Sat, 22 Jul 2006 09:22:37 +0300 (EEST)
Pekka J Enberg <penberg@cs.Helsinki.FI> wrote:

> On Thu, 20 Jul 2006 15:07:30 +0300 (EEST)
> Pekka J Enberg <penberg@cs.Helsinki.FI> wrote:
> > > This patch implements the revoke(2) and frevoke(2) system calls for all
> > > types of files.
> > >
> > > ...
> > >
> > > -	file = fget_light(fd, &fput_needed);
> > > +	file = fget(fd);
>  
> On Fri, 21 Jul 2006, Andrew Morton wrote:
> > This is sad.
> 
> There are alternatives, playing games with ->f_op, creating fake struct 
> file, and doing IS_REVOKED if-else in the paths, but I think this is by 
> far the simplest way to do it. So in the Andrew scale of sads, how 
> sad is it, exactly?-)

Sad enough.  Certainly worth an if-else to fix.

> On Thu, 20 Jul 2006 15:07:30 +0300 (EEST)
> Pekka J Enberg <penberg@cs.Helsinki.FI> wrote:
> > > +static int revoke_files(struct task_struct *owner, struct inode *inode,
> > > +			struct file *exclude, struct list_head *to_close)
> > > +{
> > > ...
> > > +	spin_lock(&files->file_lock);
> > > ...
> > > +		revoked = kmalloc(sizeof(*revoked), GFP_KERNEL);
>  
> On Fri, 21 Jul 2006, Andrew Morton wrote:
> > This is bad.
> 
> Indeed, I'll come up with a better one as soon as I sort out the mmap 
> takedown issues.
> 

Why is this approach so different from Tigran's, I wonder.

iirc, one of the things we added file.f_mapping for was revokation, but
this patch doesn't use it.  Please ask Al Viro about this.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC/PATCH] revoke/frevoke system calls
  2006-07-22  6:59     ` Andrew Morton
@ 2006-07-22  7:41       ` Pekka Enberg
  0 siblings, 0 replies; 13+ messages in thread
From: Pekka Enberg @ 2006-07-22  7:41 UTC (permalink / raw)
  To: Andrew Morton; +Cc: alan, tytso, linux-kernel, linux-fsdevel, tigran, viro

Hi,

On Sat, 22 Jul 2006 09:22:37 +0300 (EEST)
Pekka J Enberg <penberg@cs.Helsinki.FI> wrote:
> > There are alternatives, playing games with ->f_op, creating fake struct 
> > file, and doing IS_REVOKED if-else in the paths, but I think this is by 
> > far the simplest way to do it. So in the Andrew scale of sads, how 
> > sad is it, exactly?-)

On Fri, 2006-07-21 at 23:59 -0700, Andrew Morton wrote:
> Sad enough.  Certainly worth an if-else to fix.

Actually, we can fix it with file->f_light thing Tigran is doing:

http://developer.osdl.org/dev/fumount/kernel2/patches/2.6.12/1/forced-unmount-2.6.12-1.patch

On Fri, 2006-07-21 at 23:59 -0700, Andrew Morton wrote:
> Why is this approach so different from Tigran's, I wonder.

Not so different. I am blocking fork until I can revoke all open file
descriptors (i.e. substitute with NULL) whereas Tigran is dropping
tasklist_lock and retrying. I am not doing get_bad_file() because I
don't think we really need it. Tigran's mmap takedown code looks pretty
much what I want too.

On Fri, 2006-07-21 at 23:59 -0700, Andrew Morton wrote:
> iirc, one of the things we added file.f_mapping for was revokation, but
> this patch doesn't use it.  Please ask Al Viro about this.

I searched fsdevel archives but couldn't find anything on that. Al?

				Pekka


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC/PATCH] revoke/frevoke system calls
@ 2006-07-22  8:05 Albert Cahalan
  2006-08-07 10:45 ` Pekka Enberg
  0 siblings, 1 reply; 13+ messages in thread
From: Albert Cahalan @ 2006-07-22  8:05 UTC (permalink / raw)
  To: froese, B.Steinbrink, hurtta+gmane, linux-kernel, linux-fsdevel

Edgar Toernig writes:
> Bjvrn Steinbrink wrote:

>> In do_revoke() there is:
>>
>> +    if (current->fsuid != inode->i_uid && !capable(CAP_FOWNER)) {
>> +            ret = -EPERM;
>> +            goto out;
>>
>> That pretty much matches what the BSD manpage says.
>
> Urgs, so any user may remove mappings from another process and
> let it crash?

Two good solutions come to mind:

a. substitute the zero page
b. make the mapping private and touch it as if C-O-W happened

Other concerns:

Optionally excluding the current UID/TGID/TID would be good.
(some flags) A revokeat() call seems to be required. Be sure
to handle working directories. The controlling tty is special.
Flag processes with revoked ttys in /proc/*/stat please, so
that ps can report it properly without opening another file.

BTW, it is wonderful to see this happening.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC/PATCH] revoke/frevoke system calls
  2006-07-22  8:05 Albert Cahalan
@ 2006-08-07 10:45 ` Pekka Enberg
  2006-08-07 15:19   ` Albert Cahalan
  0 siblings, 1 reply; 13+ messages in thread
From: Pekka Enberg @ 2006-08-07 10:45 UTC (permalink / raw)
  To: Albert Cahalan
  Cc: froese, B.Steinbrink, hurtta+gmane, linux-kernel, linux-fsdevel

Hi,

Edgar Toernig writes:
> > Urgs, so any user may remove mappings from another process and
> > let it crash?

On 7/22/06, Albert Cahalan <acahalan@gmail.com> wrote:
> Two good solutions come to mind:
>
> a. substitute the zero page
> b. make the mapping private and touch it as if C-O-W happened

Actually, I think revokeat() and frevoke() should be consistent with
mmap which will make a process go SIGBUS if it attempts to write to
truncated shared mapping.

                                          Pekka

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC/PATCH] revoke/frevoke system calls
  2006-08-07 10:45 ` Pekka Enberg
@ 2006-08-07 15:19   ` Albert Cahalan
  0 siblings, 0 replies; 13+ messages in thread
From: Albert Cahalan @ 2006-08-07 15:19 UTC (permalink / raw)
  To: Pekka Enberg
  Cc: froese, B.Steinbrink, hurtta+gmane, linux-kernel, linux-fsdevel

On 8/7/06, Pekka Enberg <penberg@cs.helsinki.fi> wrote:
> [Albert Cahalan]
>> Edgar Toernig writes:

> > > Urgs, so any user may remove mappings from another process and
> > > let it crash?
> > Two good solutions come to mind:
> >
> > a. substitute the zero page
> > b. make the mapping private and touch it as if C-O-W happened
>
> Actually, I think revokeat() and frevoke() should be consistent with
> mmap which will make a process go SIGBUS if it attempts to write to
> truncated shared mapping.

You're right. Apps must already be tolerant of SIGBUS.
There is thus no additional risk.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2006-08-07 15:19 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-07-20 12:07 [RFC/PATCH] revoke/frevoke system calls Pekka J Enberg
2006-07-20 20:02 ` Kari Hurtta
2006-07-20 21:09   ` Björn Steinbrink
2006-07-21 22:16     ` Edgar Toernig
2006-07-22  0:19 ` Andrew Morton
2006-07-22  6:22   ` Pekka J Enberg
2006-07-22  6:59     ` Andrew Morton
2006-07-22  7:41       ` Pekka Enberg
     [not found] <6AFvY-7ZK-15@gated-at.bofh.it>
2006-07-20 21:26 ` Bodo Eggert
2006-07-20 23:01   ` roucaries bastien
  -- strict thread matches above, loose matches on Subject: below --
2006-07-22  8:05 Albert Cahalan
2006-08-07 10:45 ` Pekka Enberg
2006-08-07 15:19   ` Albert Cahalan

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).