All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Williams <dan.j.williams@intel.com>
To: viro@zeniv.linux.org.uk
Cc: "J. Bruce Fields" <bfields@fieldses.org>,
	linux-fsdevel@vger.kernel.org,
	Jeff Layton <jlayton@poochiereds.net>,
	linux-kernel@vger.kernel.org
Subject: [RFC PATCH] vfs: shutdown lease notifications on file close
Date: Fri, 13 Oct 2017 08:56:10 -0700	[thread overview]
Message-ID: <150791017031.39579.15556617763195969964.stgit@dwillia2-desk3.amr.corp.intel.com> (raw)

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@poochiereds.net>
Cc: "J. Bruce Fields" <bfields@fieldses.org>
Reported-by: Alexander Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Dan Williams <dan.j.williams@intel.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);

             reply	other threads:[~2017-10-13 16:02 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-13 15:56 Dan Williams [this message]
2017-10-13 17:01 ` [RFC PATCH] vfs: shutdown lease notifications on file close Al Viro
2017-10-13 17:43   ` Dan Williams
2017-10-13 18:00     ` Dan Williams
2017-10-13 18:30 ` Jeff Layton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=150791017031.39579.15556617763195969964.stgit@dwillia2-desk3.amr.corp.intel.com \
    --to=dan.j.williams@intel.com \
    --cc=bfields@fieldses.org \
    --cc=jlayton@poochiereds.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.