| 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
| ||
|
Message-ID: <150791017031.39579.15556617763195969964.stgit@dwillia2-desk3.amr.corp.intel.com>
Date: Fri, 13 Oct 2017 08:56:10 -0700
From: Dan Williams <dan.j.williams@...el.com>
To: viro@...iv.linux.org.uk
Cc: "J. Bruce Fields" <bfields@...ldses.org>,
linux-fsdevel@...r.kernel.org,
Jeff Layton <jlayton@...chiereds.net>,
linux-kernel@...r.kernel.org
Subject: [RFC PATCH] vfs: shutdown lease notifications on file close
While implementing MAP_DIRECT, an mmap flag that arranges for an
FL_LAYOUT lease to be established, Al noted:
You are not even guaranteed that descriptor will remain be still
open by the time you pass it down to your helper, nevermind the
moment when event actually happens...
The first problem can be solved with an fd{get,put} at mmap
{entry,exit}. The second problem appears to be a general issue.
Leases follow the lifetime of the inode, so it is possible for a lease
to be broken after the file is closed. When that happens userspace may
get a notification on a stale fd. Of course it is not recommended that a
process close a file descriptor with an active lease, but if it does we
should assume that the notification is not needed either. Walk leases at
close time and invalidate any pending fasync instances.
Cc: Jeff Layton <jlayton@...chiereds.net>
Cc: "J. Bruce Fields" <bfields@...ldses.org>
Reported-by: Alexander Viro <viro@...iv.linux.org.uk>
Signed-off-by: Dan Williams <dan.j.williams@...el.com>
---
fs/fcntl.c | 24 +++++++++++++++++++++++-
fs/file.c | 1 +
fs/locks.c | 22 ++++++++++++++++++++++
include/linux/fs.h | 7 +++++++
4 files changed, 53 insertions(+), 1 deletion(-)
diff --git a/fs/fcntl.c b/fs/fcntl.c
index 448a1119f0be..03612c363b90 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -974,6 +974,28 @@ int fasync_helper(int fd, struct file * filp, int on, struct fasync_struct **fap
EXPORT_SYMBOL(fasync_helper);
+static void __fasync_silence(int fd, struct fasync_struct *fa)
+{
+ while (fa) {
+ unsigned long flags;
+
+ spin_lock_irqsave(&fa->fa_lock, flags);
+ if (fa->fa_file && fa->fa_fd == fd)
+ fa->fa_fd = -1;
+ spin_unlock_irqrestore(&fa->fa_lock, flags);
+ fa = rcu_dereference(fa->fa_next);
+ }
+}
+
+void fasync_silence(int fd, struct fasync_struct **fp)
+{
+ if (*fp) {
+ rcu_read_lock();
+ __fasync_silence(fd, *fp);
+ rcu_read_unlock();
+ }
+}
+
/*
* rcu_read_lock() is held
*/
@@ -989,7 +1011,7 @@ static void kill_fasync_rcu(struct fasync_struct *fa, int sig, int band)
return;
}
spin_lock_irqsave(&fa->fa_lock, flags);
- if (fa->fa_file) {
+ if (fa->fa_file && fa->fa_fd >= 0) {
fown = &fa->fa_file->f_owner;
/* Don't send SIGURG to processes which have not set a
queued signum: SIGURG has its own default signalling
diff --git a/fs/file.c b/fs/file.c
index 1fc7fbbb4510..b90969bf1f94 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -633,6 +633,7 @@ int __close_fd(struct files_struct *files, unsigned fd)
rcu_assign_pointer(fdt->fd[fd], NULL);
__clear_close_on_exec(fd, fdt);
__put_unused_fd(files, fd);
+ locks_silence_lease(fd, file);
spin_unlock(&files->file_lock);
return filp_close(file, files);
diff --git a/fs/locks.c b/fs/locks.c
index 1bd71c4d663a..ca93e4dbdd90 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -2573,6 +2573,28 @@ void locks_remove_file(struct file *filp)
spin_unlock(&ctx->flc_lock);
}
+/*
+ * The fd is assumed to valid, i.e. this routine is called in the
+ * filp_close() path where the state of fd is known.
+ */
+void locks_silence_lease(int fd, struct file *filp)
+{
+ struct file_lock_context *ctx;
+ struct file_lock *fl;
+
+ ctx = smp_load_acquire(&locks_inode(filp)->i_flctx);
+ if (!ctx || list_empty_careful(&ctx->flc_lease))
+ return;
+
+ spin_lock(&ctx->flc_lock);
+ list_for_each_entry(fl, &ctx->flc_lease, fl_list) {
+ if (fl->fl_pid != current->tgid)
+ continue;
+ fasync_silence(fd, &fl->fl_fasync);
+ }
+ spin_unlock(&ctx->flc_lock);
+}
+
/**
* posix_unblock_lock - stop waiting for a file lock
* @waiter: the lock which was waiting
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 17e0e899e184..019853a7b2cd 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1076,6 +1076,7 @@ extern void locks_copy_lock(struct file_lock *, struct file_lock *);
extern void locks_copy_conflock(struct file_lock *, struct file_lock *);
extern void locks_remove_posix(struct file *, fl_owner_t);
extern void locks_remove_file(struct file *);
+extern void locks_silence_lease(int, struct file *);
extern void locks_release_private(struct file_lock *);
extern void posix_test_lock(struct file *, struct file_lock *);
extern int posix_lock_file(struct file *, struct file_lock *, struct file_lock *);
@@ -1153,6 +1154,11 @@ static inline void locks_remove_posix(struct file *filp, fl_owner_t owner)
return;
}
+static inline void locks_silence_lease(int fd, struct file *filp)
+{
+ return;
+}
+
static inline void locks_remove_file(struct file *filp)
{
return;
@@ -1260,6 +1266,7 @@ extern struct fasync_struct *fasync_insert_entry(int, struct file *, struct fasy
extern int fasync_remove_entry(struct file *, struct fasync_struct **);
extern struct fasync_struct *fasync_alloc(void);
extern void fasync_free(struct fasync_struct *);
+extern void fasync_silence(int, struct fasync_struct **);
/* can be called from interrupts */
extern void kill_fasync(struct fasync_struct **, int, int);
Powered by blists - more mailing lists