Re: [PATCH] revoke: delayed file closing
On 3/9/07, Pekka J Enberg <[EMAIL PROTECTED]> wrote: To fix this, change sys_revoke() not to close the actual revoked file immediately. Instead, we do filp_close() when the user does close(2) on the revoked file descriptor. Btw, this is safe because a filesystem implementing f_ops->revoke must ensure there are no pending writes after it has completed so f_ops->flush will not do any actual flushing when it is invoked by a delayed close. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] revoke: delayed file closing
From: Pekka Enberg <[EMAIL PROTECTED]>
As explained by Eric Dumazet, one of the interests of fget_light() is
to avoid dirtying struct file which is broken by the newly added
file->f_light. In addition, fget_light() currently has a race window
between fcheck_files() and set_f_light().
To fix this, change sys_revoke() not to close the actual revoked file
immediately. Instead, we do filp_close() when the user does close(2)
on the revoked file descriptor.
Cc: Eric Dumazet <[EMAIL PROTECTED]>
Signed-off-by: Pekka Enberg <[EMAIL PROTECTED]>
---
fs/file_table.c |1
fs/revoke.c | 53 +++
fs/revoked_inode.c |3 +-
include/linux/file.h | 13 --
include/linux/fs.h |2 -
include/linux/revoked_fs_i.h | 20
6 files changed, 26 insertions(+), 66 deletions(-)
Index: uml-2.6/fs/file_table.c
===
--- uml-2.6.orig/fs/file_table.c2007-03-09 14:06:48.0 +0200
+++ uml-2.6/fs/file_table.c 2007-03-09 14:06:53.0 +0200
@@ -219,7 +219,6 @@
*fput_needed = 0;
if (likely((atomic_read(>count) == 1))) {
file = fcheck_files(files, fd);
- set_f_light(file);
} else {
rcu_read_lock();
file = fcheck_files(files, fd);
Index: uml-2.6/fs/revoke.c
===
--- uml-2.6.orig/fs/revoke.c2007-03-09 14:00:02.0 +0200
+++ uml-2.6/fs/revoke.c 2007-03-09 14:18:15.0 +0200
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
/*
* This is used for pre-allocating an array of file pointers so that we don't
@@ -33,20 +34,6 @@
*/
static struct vfsmount *revokefs_mnt;
-struct revokefs_inode_info {
- struct task_struct *owner;
- struct file *file;
- unsigned int fd;
- struct inode vfs_inode;
-};
-
-static inline struct revokefs_inode_info *revokefs_i(struct inode *inode)
-{
- return container_of(inode, struct revokefs_inode_info, vfs_inode);
-}
-
-extern void make_revoked_inode(struct inode *, int);
-
static struct file *get_revoked_file(void)
{
struct dentry *dentry;
@@ -235,24 +222,6 @@
return err;
}
-static int task_filp_close(struct task_struct *task, struct file *filp)
-{
- struct files_struct *files;
- int err = 0;
-
- files = get_files_struct(task);
- if (files) {
- /*
-* Wait until sys_read and sys_write are done.
-*/
- while (filp->f_light)
- schedule();
- err = filp_close(filp, files);
- put_files_struct(files);
- }
- return err;
-}
-
static void restore_file(struct revokefs_inode_info *info)
{
struct files_struct *files;
@@ -293,19 +262,6 @@
}
}
-static int revoke_file(struct task_struct *task, struct file *filp)
-{
- int err;
-
- err = filp->f_op->revoke(filp);
- if (err)
- goto out;
-
- err = task_filp_close(task, filp);
- out:
- return err;
-}
-
static int revoke_files(struct revoke_table *table)
{
unsigned long i;
@@ -313,7 +269,7 @@
for (i = 0; i < table->end; i++) {
struct revokefs_inode_info *info;
- struct file *this;
+ struct file *this, *filp;
this = table->files[i];
info = revokefs_i(this->f_dentry->d_inode);
@@ -323,7 +279,8 @@
* an partially closed file can no longer be restored.
*/
table->restore_start++;
- err = revoke_file(info->owner, info->file);
+ filp = info->file;
+ err = filp->f_op->revoke(filp);
put_task_struct(info->owner);
info->owner = NULL; /* To avoid restoring closed file. */
if (err)
@@ -565,8 +522,6 @@
kmem_cache_free(revokefs_inode_cache, revokefs_i(inode));
}
-#define REVOKEFS_MAGIC 0x5245564B /* REVK */
-
static struct super_operations revokefs_super_ops = {
.alloc_inode = revokefs_alloc_inode,
.destroy_inode = revokefs_destroy_inode,
Index: uml-2.6/fs/revoked_inode.c
===
--- uml-2.6.orig/fs/revoked_inode.c 2007-03-09 14:03:58.0 +0200
+++ uml-2.6/fs/revoked_inode.c 2007-03-09 14:05:21.0 +0200
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
static loff_t revoked_file_llseek(struct file *file, loff_t offset, int origin)
{
@@ -96,7 +97,7 @@
static int revoked_file_flush(struct file *file, fl_owner_t id)
{
- return 0;
+ return filp_close(file, id);
}
static int revoked_file_release(struct inode *inode, struct file *filp)
Index: uml-2.6/include/linux/file.h
[PATCH] revoke: delayed file closing
From: Pekka Enberg [EMAIL PROTECTED]
As explained by Eric Dumazet, one of the interests of fget_light() is
to avoid dirtying struct file which is broken by the newly added
file-f_light. In addition, fget_light() currently has a race window
between fcheck_files() and set_f_light().
To fix this, change sys_revoke() not to close the actual revoked file
immediately. Instead, we do filp_close() when the user does close(2)
on the revoked file descriptor.
Cc: Eric Dumazet [EMAIL PROTECTED]
Signed-off-by: Pekka Enberg [EMAIL PROTECTED]
---
fs/file_table.c |1
fs/revoke.c | 53 +++
fs/revoked_inode.c |3 +-
include/linux/file.h | 13 --
include/linux/fs.h |2 -
include/linux/revoked_fs_i.h | 20
6 files changed, 26 insertions(+), 66 deletions(-)
Index: uml-2.6/fs/file_table.c
===
--- uml-2.6.orig/fs/file_table.c2007-03-09 14:06:48.0 +0200
+++ uml-2.6/fs/file_table.c 2007-03-09 14:06:53.0 +0200
@@ -219,7 +219,6 @@
*fput_needed = 0;
if (likely((atomic_read(files-count) == 1))) {
file = fcheck_files(files, fd);
- set_f_light(file);
} else {
rcu_read_lock();
file = fcheck_files(files, fd);
Index: uml-2.6/fs/revoke.c
===
--- uml-2.6.orig/fs/revoke.c2007-03-09 14:00:02.0 +0200
+++ uml-2.6/fs/revoke.c 2007-03-09 14:18:15.0 +0200
@@ -14,6 +14,7 @@
#include linux/module.h
#include linux/mount.h
#include linux/sched.h
+#include linux/revoked_fs_i.h
/*
* This is used for pre-allocating an array of file pointers so that we don't
@@ -33,20 +34,6 @@
*/
static struct vfsmount *revokefs_mnt;
-struct revokefs_inode_info {
- struct task_struct *owner;
- struct file *file;
- unsigned int fd;
- struct inode vfs_inode;
-};
-
-static inline struct revokefs_inode_info *revokefs_i(struct inode *inode)
-{
- return container_of(inode, struct revokefs_inode_info, vfs_inode);
-}
-
-extern void make_revoked_inode(struct inode *, int);
-
static struct file *get_revoked_file(void)
{
struct dentry *dentry;
@@ -235,24 +222,6 @@
return err;
}
-static int task_filp_close(struct task_struct *task, struct file *filp)
-{
- struct files_struct *files;
- int err = 0;
-
- files = get_files_struct(task);
- if (files) {
- /*
-* Wait until sys_read and sys_write are done.
-*/
- while (filp-f_light)
- schedule();
- err = filp_close(filp, files);
- put_files_struct(files);
- }
- return err;
-}
-
static void restore_file(struct revokefs_inode_info *info)
{
struct files_struct *files;
@@ -293,19 +262,6 @@
}
}
-static int revoke_file(struct task_struct *task, struct file *filp)
-{
- int err;
-
- err = filp-f_op-revoke(filp);
- if (err)
- goto out;
-
- err = task_filp_close(task, filp);
- out:
- return err;
-}
-
static int revoke_files(struct revoke_table *table)
{
unsigned long i;
@@ -313,7 +269,7 @@
for (i = 0; i table-end; i++) {
struct revokefs_inode_info *info;
- struct file *this;
+ struct file *this, *filp;
this = table-files[i];
info = revokefs_i(this-f_dentry-d_inode);
@@ -323,7 +279,8 @@
* an partially closed file can no longer be restored.
*/
table-restore_start++;
- err = revoke_file(info-owner, info-file);
+ filp = info-file;
+ err = filp-f_op-revoke(filp);
put_task_struct(info-owner);
info-owner = NULL; /* To avoid restoring closed file. */
if (err)
@@ -565,8 +522,6 @@
kmem_cache_free(revokefs_inode_cache, revokefs_i(inode));
}
-#define REVOKEFS_MAGIC 0x5245564B /* REVK */
-
static struct super_operations revokefs_super_ops = {
.alloc_inode = revokefs_alloc_inode,
.destroy_inode = revokefs_destroy_inode,
Index: uml-2.6/fs/revoked_inode.c
===
--- uml-2.6.orig/fs/revoked_inode.c 2007-03-09 14:03:58.0 +0200
+++ uml-2.6/fs/revoked_inode.c 2007-03-09 14:05:21.0 +0200
@@ -17,6 +17,7 @@
#include linux/smp_lock.h
#include linux/namei.h
#include linux/poll.h
+#include linux/revoked_fs_i.h
static loff_t revoked_file_llseek(struct file *file, loff_t offset, int origin)
{
@@ -96,7 +97,7 @@
static int revoked_file_flush(struct file *file, fl_owner_t id)
{
- return 0;
+ return filp_close(file, id);
}
static int
Re: [PATCH] revoke: delayed file closing
On 3/9/07, Pekka J Enberg [EMAIL PROTECTED] wrote: To fix this, change sys_revoke() not to close the actual revoked file immediately. Instead, we do filp_close() when the user does close(2) on the revoked file descriptor. Btw, this is safe because a filesystem implementing f_ops-revoke must ensure there are no pending writes after it has completed so f_ops-flush will not do any actual flushing when it is invoked by a delayed close. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
