linux-kernel - [RFC PATCH] dup3(): option to detect close() error

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:	Wed, 02 Jul 2014 03:52:56 +0300
From:	Nominal Animal <kernel@...inal-animal.net>
To:	linux-fsdevel@...r.kernel.org
CC:	linux-kernel@...r.kernel.org,
	Alexander Viro <viro@...iv.linux.org.uk>
Subject: [RFC PATCH] dup3(): option to detect close() error

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)) {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/