mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Nominal Animal <kernel@nominal-animal.net>
To: linux-fsdevel@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Alexander Viro <viro@zeniv.linux.org.uk>
Subject: [RFC PATCH] dup3(): option to detect close() error
Date: Wed, 02 Jul 2014 03:52:56 +0300	[thread overview]
Message-ID: <53B357E8.1090909@nominal-animal.net> (raw)

The RFC patch below adds a new flag for dup3().
(For simplicity, I'm just reusing O_EXCL for now.)

I am assuming calling f_op->flush() twice instead of just once
before closing the file is not harmful.

The idea is to extend dup3() so that we can catch close() errors
for the descriptor to be replaced, if we want to. Existing code
will not see any changes. (Userspace might wish to replace a
descriptor exactly because there is a problem, so we don't want
to make this unconditional.

This approach relies on close() having a single error path in Linux:
the file has an f_op->flush() handler, and it returns an error.
Here, if the flag is included in dup3(), f_op->flush() is called early
for newfd, and if it fails, dup3() will return with -EIO.

It will still be called again in filp_close(), but since the descriptor
is to be closed, there should be no activity between the two
f_op->flush() calls, so the latter should not fail.

Is there a reason (besides having yet another O_ flag) why this
would not work, or why we would not want to do this?
Am I missing something?
I just want be able to know if a problem was detected, that's all.

Best,
    Nominal Animal

diff -u5 -bar linux-3.16-rc3/fs/file.c linux-3.16-rc3.new/fs/file.c
--- linux-3.16-rc3/fs/file.c	2014-06-30 00:11:36.000000000 +0300
+++ linux-3.16-rc3.new/fs/file.c	2014-07-02 02:04:23.708114821 +0300
@@ -816,21 +816,31 @@
 SYSCALL_DEFINE3(dup3, unsigned int, oldfd, unsigned int, newfd, int, flags)
 {
 	int err = -EBADF;
-	struct file *file;
+	struct file *file, *tofree;
 	struct files_struct *files = current->files;
 
-	if ((flags & ~O_CLOEXEC) != 0)
+	if ((flags & ~(O_CLOEXEC | O_EXCL)) != 0)
 		return -EINVAL;
 
 	if (unlikely(oldfd == newfd))
 		return -EINVAL;
 
 	if (newfd >= rlimit(RLIMIT_NOFILE))
 		return -EBADF;
 
 	spin_lock(&files->file_lock);
+	if (unlikely(flags & O_EXCL)) {
+		tofree = fcheck(newfd);
+		if (tofree && file_count(tofree) && tofree->f_op->flush) {
+			spin_unlock(&files->file_lock);
+			err = tofree->f_op->flush(tofree, files);
+			if (unlikely(err < 0))
+				return -EIO;
+			spin_lock(&files->file_lock);
+		}
+	}
 	err = expand_files(files, newfd);
 	file = fcheck(oldfd);
 	if (unlikely(!file))
 		goto Ebadf;
 	if (unlikely(err < 0)) {

                 reply	other threads:[~2014-07-02  1:09 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=53B357E8.1090909@nominal-animal.net \
    --to=kernel@nominal-animal.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®