linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@amacapital.net>
To: Al Viro <viro@zeniv.linux.org.uk>,
	Linus Torvalds <torvalds@linux-foundation.org>
Cc: Willy Tarreau <w@1wt.eu>, Ingo Molnar <mingo@kernel.org>,
	"security@kernel.org" <security@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Oleg Nesterov <oleg@redhat.com>,
	Linux FS Devel <linux-fsdevel@vger.kernel.org>,
	Brad Spengler <spender@grsecurity.net>,
	Andy Lutomirski <luto@amacapital.net>
Subject: [RFC PATCH] fs: Add user_file_or_path_at and use it for truncate
Date: Tue, 27 Aug 2013 12:16:34 -0700	[thread overview]
Message-ID: <7d1419dda1da70a8ad915f85b093a58b86bcaf3b.1377630856.git.luto@amacapital.net> (raw)
In-Reply-To: <CA+55aFw20W9L8HV+VbTRNeQQM-_+aeUhvT9AAod5LGfb-7hE9g@mail.gmail.com>

This is an experiment to see if we can get nice semantics for all syscalls
that either follow symlinks or allow AT_EMPTY_PATH without jumping through
enormous hoops.  This converts truncate (although you can't tell using
truncate from coreutils, because it actually uses open + ftruncate).

The basic idea is that there's a new helper function
user_file_or_path_at.  It takes an fd and a path and, depending on
flags, the emptiness of the path, and whether path is a magic /proc
symlink (or a symlink to a magic /proc/symlink), it returns either a
struct path or a struct file *.

To make this genuinely useful, something similar will need to happen for
open/openat.

The lifetime rules for the contents of struct nameidata seem weird.
If all users first memset the thing to zero and called nd_put or
something when they were done, this would be cleaner.  But they don't.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 fs/namei.c            | 73 ++++++++++++++++++++++++++++++++++++++++
 fs/open.c             | 93 +++++++++++++++++++++++++++++----------------------
 fs/proc/base.c        | 44 +++++++++++++-----------
 fs/proc/fd.c          | 10 +++---
 fs/proc/internal.h    |  3 +-
 include/linux/namei.h | 14 ++++++++
 6 files changed, 171 insertions(+), 66 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 89a612e..0d905ac 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -691,6 +691,28 @@ void nd_jump_link(struct nameidata *nd, struct path *path)
 	nd->flags |= LOOKUP_JUMPED;
 }
 
+/*
+ * Helper for /proc/pid/fd/N-style magic links.  This is like nd_jump_link
+ * except that it will apply additional security checks.  Caller must have
+ * taken a reference to file beforehand.
+ */
+void nd_jump_link_file(struct nameidata *nd, struct file *file)
+{
+	path_put(&nd->path);
+
+	nd->path = file->f_path;
+	path_get(&nd->path);
+	if (nd->flags & LOOKUP_FILE) {
+		if (nd->last_symlink_file)
+			fput(nd->last_symlink_file);
+		nd->last_symlink_file = file;
+	} else {
+		fput(file);
+	}
+	nd->inode = nd->path.dentry->d_inode;
+	nd->flags |= LOOKUP_JUMPED;
+}
+
 static inline void put_link(struct nameidata *nd, struct path *link, void *cookie)
 {
 	struct inode *inode = link->dentry->d_inode;
@@ -842,6 +864,10 @@ follow_link(struct path *link, struct nameidata *nd, void **p)
 		goto out_put_nd_path;
 
 	nd->last_type = LAST_BIND;
+	if (unlikely((nd->flags & LOOKUP_FILE) && nd->last_symlink_file)) {
+		fput(nd->last_symlink_file);
+		nd->last_symlink_file = NULL;
+	}
 	*p = dentry->d_inode->i_op->follow_link(dentry, nd);
 	error = PTR_ERR(*p);
 	if (IS_ERR(*p))
@@ -2156,6 +2182,53 @@ int user_path_at(int dfd, const char __user *name, unsigned flags,
 	return user_path_at_empty(dfd, name, flags, path, NULL);
 }
 
+int user_file_or_path_at(int dfd, const char __user *filename,
+			 int lookup_flags, bool null_filename_okay,
+			 struct file_or_path *out)
+{
+	struct nameidata nd;
+	struct filename *tmp;
+	int err;
+	int empty = 0;
+
+	if (!filename && null_filename_okay) {
+		out->file = fget(dfd);
+		return out->file ? 0 : -EBADF;
+	}
+
+	tmp = getname_flags(filename, lookup_flags, &empty);
+	if (IS_ERR(tmp))
+		return PTR_ERR(tmp);
+
+	if (empty) {
+		out->file = fget(dfd);
+		return out->file ? 0 : -EBADF;
+	}
+
+	nd.last_symlink_file = NULL;
+	BUG_ON(lookup_flags & LOOKUP_PARENT);
+
+	err = filename_lookup(dfd, tmp, lookup_flags | LOOKUP_FILE, &nd);
+	putname(tmp);
+	if (!err) {
+		if (nd.last_symlink_file && nd.last_type == LAST_BIND) {
+			out->file = nd.last_symlink_file;
+			memset(&out->path, 0, sizeof(out->path));
+			path_put(&nd.path);
+		} else {
+			out->path = nd.path;
+			out->file = NULL;
+			if (nd.last_symlink_file)
+				fput(nd.last_symlink_file);
+		}
+	} else {
+		if (nd.last_symlink_file)
+			fput(nd.last_symlink_file);
+	}
+
+	return err;
+}
+
 /*
  * NB: most callers don't do anything directly with the reference to the
  *     to struct filename, but the nd->last pointer points into the name string
diff --git a/fs/open.c b/fs/open.c
index 7931f76..1349b43 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -114,20 +114,66 @@ out:
 }
 EXPORT_SYMBOL_GPL(vfs_truncate);
 
+static long do_truncate_file(struct file *file, loff_t length, int small)
+{
+	struct inode *inode;
+	struct dentry *dentry;
+	int error;
+
+	error = -EINVAL;
+	if (length < 0)
+		goto out;
+
+	/* explicitly opened as large or we are on 64-bit box */
+	if (file->f_flags & O_LARGEFILE)
+		small = 0;
+
+	dentry = file->f_path.dentry;
+	inode = dentry->d_inode;
+	error = -EINVAL;
+	if (!S_ISREG(inode->i_mode) || !(file->f_mode & FMODE_WRITE))
+		goto out;
+
+	error = -EINVAL;
+	/* Cannot ftruncate over 2^31 bytes without large file support */
+	if (small && length > MAX_NON_LFS)
+		goto out;
+
+	error = -EPERM;
+	if (IS_APPEND(inode))
+		goto out;
+
+	sb_start_write(inode->i_sb);
+	error = locks_verify_truncate(inode, file, length);
+	if (!error)
+		error = security_path_truncate(&file->f_path);
+	if (!error)
+		error = do_truncate(dentry, length, ATTR_MTIME|ATTR_CTIME, file);
+	sb_end_write(inode->i_sb);
+out:
+	return error;
+}
+
 static long do_sys_truncate(const char __user *pathname, loff_t length)
 {
-	unsigned int lookup_flags = LOOKUP_FOLLOW;
-	struct path path;
+	struct file_or_path obj;
+	int lookup_flags = LOOKUP_FOLLOW;
 	int error;
 
 	if (length < 0)	/* sorry, but loff_t says... */
 		return -EINVAL;
 
 retry:
-	error = user_path_at(AT_FDCWD, pathname, lookup_flags, &path);
+	error = user_file_or_path_at(AT_FDCWD, pathname,
+				     LOOKUP_FOLLOW, false, &obj);
 	if (!error) {
-		error = vfs_truncate(&path, length);
-		path_put(&path);
+		if (obj.file) {
+			error = do_truncate_file(obj.file, length, 0);
+			fput(obj.file);
+		} else {
+			error = vfs_truncate(&obj.path, length);
+			path_put(&obj.path);
+		}
 	}
 	if (retry_estale(error, lookup_flags)) {
 		lookup_flags |= LOOKUP_REVAL;
@@ -150,48 +196,15 @@ COMPAT_SYSCALL_DEFINE2(truncate, const char __user *, path, compat_off_t, length
 
 static long do_sys_ftruncate(unsigned int fd, loff_t length, int small)
 {
-	struct inode *inode;
-	struct dentry *dentry;
 	struct fd f;
 	int error;
 
-	error = -EINVAL;
-	if (length < 0)
-		goto out;
 	error = -EBADF;
 	f = fdget(fd);
 	if (!f.file)
-		goto out;
-
-	/* explicitly opened as large or we are on 64-bit box */
-	if (f.file->f_flags & O_LARGEFILE)
-		small = 0;
-
-	dentry = f.file->f_path.dentry;
-	inode = dentry->d_inode;
-	error = -EINVAL;
-	if (!S_ISREG(inode->i_mode) || !(f.file->f_mode & FMODE_WRITE))
-		goto out_putf;
-
-	error = -EINVAL;
-	/* Cannot ftruncate over 2^31 bytes without large file support */
-	if (small && length > MAX_NON_LFS)
-		goto out_putf;
-
-	error = -EPERM;
-	if (IS_APPEND(inode))
-		goto out_putf;
-
-	sb_start_write(inode->i_sb);
-	error = locks_verify_truncate(inode, f.file, length);
-	if (!error)
-		error = security_path_truncate(&f.file->f_path);
-	if (!error)
-		error = do_truncate(dentry, length, ATTR_MTIME|ATTR_CTIME, f.file);
-	sb_end_write(inode->i_sb);
-out_putf:
+		return error;
+	error = do_truncate_file(f.file, length, small);
 	fdput(f);
-out:
 	return error;
 }
 
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 1485e38..ac28ff3 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -171,7 +171,7 @@ static int get_task_root(struct task_struct *task, struct path *root)
 	return result;
 }
 
-static int proc_cwd_link(struct dentry *dentry, struct path *path)
+static int proc_cwd_link(struct dentry *dentry, struct file_or_path *link)
 {
 	struct task_struct *task = get_proc_task(dentry->d_inode);
 	int result = -ENOENT;
@@ -179,7 +179,8 @@ static int proc_cwd_link(struct dentry *dentry, struct path *path)
 	if (task) {
 		task_lock(task);
 		if (task->fs) {
-			get_fs_pwd(task->fs, path);
+			link->file = NULL;
+			get_fs_pwd(task->fs, &link->path);
 			result = 0;
 		}
 		task_unlock(task);
@@ -188,13 +189,14 @@ static int proc_cwd_link(struct dentry *dentry, struct path *path)
 	return result;
 }
 
-static int proc_root_link(struct dentry *dentry, struct path *path)
+static int proc_root_link(struct dentry *dentry, struct file_or_path *link)
 {
 	struct task_struct *task = get_proc_task(dentry->d_inode);
 	int result = -ENOENT;
 
 	if (task) {
-		result = get_task_root(task, path);
+		link->file = NULL;
+		result = get_task_root(task, &link->path);
 		put_task_struct(task);
 	}
 	return result;
@@ -1430,11 +1432,10 @@ static const struct file_operations proc_pid_set_comm_operations = {
 	.release	= single_release,
 };
 
-static int proc_exe_link(struct dentry *dentry, struct path *exe_path)
+static int proc_exe_link(struct dentry *dentry, struct file_or_path *link)
 {
 	struct task_struct *task;
 	struct mm_struct *mm;
-	struct file *exe_file;
 
 	task = get_proc_task(dentry->d_inode);
 	if (!task)
@@ -1443,32 +1444,32 @@ static int proc_exe_link(struct dentry *dentry, struct path *exe_path)
 	put_task_struct(task);
 	if (!mm)
 		return -ENOENT;
-	exe_file = get_mm_exe_file(mm);
+	link->file = get_mm_exe_file(mm);
 	mmput(mm);
-	if (exe_file) {
-		*exe_path = exe_file->f_path;
-		path_get(&exe_file->f_path);
-		fput(exe_file);
+	if (link->file)
 		return 0;
-	} else
+	else
 		return -ENOENT;
 }
 
 static void *proc_pid_follow_link(struct dentry *dentry, struct nameidata *nd)
 {
 	struct inode *inode = dentry->d_inode;
-	struct path path;
+	struct file_or_path link;
 	int error = -EACCES;
 
 	/* Are we allowed to snoop on the tasks file descriptors? */
 	if (!proc_fd_access_allowed(inode))
 		goto out;
 
-	error = PROC_I(inode)->op.proc_get_link(dentry, &path);
+	error = PROC_I(inode)->op.proc_get_link(dentry, &link);
 	if (error)
 		goto out;
 
-	nd_jump_link(nd, &path);
+	if (link.file)
+		nd_jump_link_file(nd, link.file);
+	else
+		nd_jump_link(nd, &link.path);
 	return NULL;
 out:
 	return ERR_PTR(error);
@@ -1502,18 +1503,23 @@ static int proc_pid_readlink(struct dentry * dentry, char __user * buffer, int b
 {
 	int error = -EACCES;
 	struct inode *inode = dentry->d_inode;
-	struct path path;
+	struct file_or_path link;
 
 	/* Are we allowed to snoop on the tasks file descriptors? */
 	if (!proc_fd_access_allowed(inode))
 		goto out;
 
-	error = PROC_I(inode)->op.proc_get_link(dentry, &path);
+	error = PROC_I(inode)->op.proc_get_link(dentry, &link);
 	if (error)
 		goto out;
 
-	error = do_proc_readlink(&path, buffer, buflen);
-	path_put(&path);
+	if (link.file) {
+		error = do_proc_readlink(&link.file->f_path, buffer, buflen);
+		fput(link.file);
+	} else {
+		error = do_proc_readlink(&link.path, buffer, buflen);
+		path_put(&link.path);
+	}
 out:
 	return error;
 }
diff --git a/fs/proc/fd.c b/fs/proc/fd.c
index 0ff80f9..4c88fc2 100644
--- a/fs/proc/fd.c
+++ b/fs/proc/fd.c
@@ -137,7 +137,7 @@ static const struct dentry_operations tid_fd_dentry_operations = {
 	.d_delete	= pid_delete_dentry,
 };
 
-static int proc_fd_link(struct dentry *dentry, struct path *path)
+static int proc_fd_link(struct dentry *dentry, struct file_or_path *link)
 {
 	struct files_struct *files = NULL;
 	struct task_struct *task;
@@ -151,13 +151,11 @@ static int proc_fd_link(struct dentry *dentry, struct path *path)
 
 	if (files) {
 		int fd = proc_fd(dentry->d_inode);
-		struct file *fd_file;
 
 		spin_lock(&files->file_lock);
-		fd_file = fcheck_files(files, fd);
-		if (fd_file) {
-			*path = fd_file->f_path;
-			path_get(&fd_file->f_path);
+		link->file = fcheck_files(files, fd);
+		if (link->file) {
+			get_file(link->file);
 			ret = 0;
 		}
 		spin_unlock(&files->file_lock);
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index 651d09a..fc936d5 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -14,6 +14,7 @@
 #include <linux/spinlock.h>
 #include <linux/atomic.h>
 #include <linux/binfmts.h>
+#include <linux/namei.h>
 
 struct ctl_table_header;
 struct mempolicy;
@@ -51,7 +52,7 @@ struct proc_dir_entry {
 };
 
 union proc_op {
-	int (*proc_get_link)(struct dentry *, struct path *);
+	int (*proc_get_link)(struct dentry *, struct file_or_path *link);
 	int (*proc_read)(struct task_struct *task, char *page);
 	int (*proc_show)(struct seq_file *m,
 		struct pid_namespace *ns, struct pid *pid,
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 5a5ff57..77f585e 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -15,6 +15,7 @@ struct nameidata {
 	struct qstr	last;
 	struct path	root;
 	struct inode	*inode; /* path.dentry.d_inode */
+	struct file	*last_symlink_file;
 	unsigned int	flags;
 	unsigned	seq;
 	int		last_type;
@@ -56,9 +57,21 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
 #define LOOKUP_ROOT		0x2000
 #define LOOKUP_EMPTY		0x4000
 
+#define LOOKUP_FILE		0x8000
+
 extern int user_path_at(int, const char __user *, unsigned, struct path *);
 extern int user_path_at_empty(int, const char __user *, unsigned, struct path *, int *empty);
 
+struct file_or_path
+{
+	struct file *file;
+	struct path path;
+};
+
+extern int user_file_or_path_at(int dfd, const char __user *filename,
+				int lookup_flags, bool null_filename_okay,
+				struct file_or_path *out);
+
 #define user_path(name, path) user_path_at(AT_FDCWD, name, LOOKUP_FOLLOW, path)
 #define user_lpath(name, path) user_path_at(AT_FDCWD, name, 0, path)
 #define user_path_dir(name, path) \
@@ -83,6 +96,7 @@ extern struct dentry *lock_rename(struct dentry *, struct dentry *);
 extern void unlock_rename(struct dentry *, struct dentry *);
 
 extern void nd_jump_link(struct nameidata *nd, struct path *path);
+extern void nd_jump_link_file(struct nameidata *nd, struct file *file);
 
 static inline void nd_set_link(struct nameidata *nd, char *path)
 {
-- 
1.8.3.1

  parent reply	other threads:[~2013-08-27 19:16 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-21 19:14 [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH) Andy Lutomirski
     [not found] ` <CA+55aFxi-ps2f2M8xPhfbuQ0pToqupPrDsLi2+GPUK2sqdYfUw@mail.gmail.com>
     [not found]   ` <CALCETrW7+LcexA6v6RQDKhni_yJAduOmiSDneCpq3v8sPDvwUQ@mail.gmail.com>
2013-08-21 20:16     ` Willy Tarreau
2013-08-22 18:48 ` Linus Torvalds
2013-08-22 18:53   ` Willy Tarreau
2013-08-22 19:05     ` Andy Lutomirski
2013-08-22 19:23       ` Linus Torvalds
2013-08-22 20:10         ` Andy Lutomirski
2013-08-22 20:15           ` Willy Tarreau
2013-08-22 20:22             ` Andy Lutomirski
2013-08-22 20:44               ` Linus Torvalds
2013-08-22 20:48                 ` Andy Lutomirski
2013-08-22 20:54                   ` Linus Torvalds
2013-08-22 20:58                     ` Andy Lutomirski
2013-08-23  1:07                     ` Al Viro
2013-08-25  3:37                       ` Al Viro
2013-08-25  7:26                         ` Andy Lutomirski
2013-08-25 14:23                           ` Al Viro
2013-08-25 17:04                             ` Andy Lutomirski
2013-08-25 19:57                         ` Linus Torvalds
2013-08-25 20:06                           ` Al Viro
2013-08-25 20:23                             ` Linus Torvalds
2013-08-26 17:37                               ` Linus Torvalds
2013-08-26 18:07                                 ` Linus Torvalds
2013-08-26 18:11                                   ` Andy Lutomirski
2013-08-27 19:16                                   ` Andy Lutomirski [this message]
2013-08-27 19:32                                     ` [RFC PATCH] fs: Add user_file_or_path_at and use it for truncate Linus Torvalds
2013-08-27 20:28                                       ` Andy Lutomirski
2013-08-28  6:16                                         ` Al Viro
2013-08-28 16:24                                           ` Linus Torvalds
2013-08-28 19:04                                           ` Andy Lutomirski
2013-08-28 19:59                                             ` Al Viro
2013-08-28 21:07                                               ` Andy Lutomirski
2013-08-27 23:08                                     ` Al Viro
2013-08-27 23:13                                       ` Andy Lutomirski
2013-08-24 18:29             ` /proc/pid/fd && anon_inode_fops Oleg Nesterov
2013-08-24 21:24               ` Willy Tarreau
2013-08-25  5:23                 ` Al Viro
2013-08-25  6:50                   ` Willy Tarreau
2013-08-25 18:51                     ` Linus Torvalds
2013-08-25 19:48                       ` Oleg Nesterov
2013-08-25 20:05                         ` Linus Torvalds
2013-08-26 15:33                           ` Oleg Nesterov
2013-08-26 16:37                             ` Oleg Nesterov
2013-08-26 17:54                               ` [PATCH] proc: make proc_fd_permission() thread-friendly Oleg Nesterov
2013-08-26 18:09                                 ` Linus Torvalds
2013-08-26 19:35                                   ` Linus Torvalds
2013-08-26 20:20                                     ` Willy Tarreau
2013-08-27 15:05                                       ` Oleg Nesterov
2013-08-27 14:39                                     ` [PATCH 0/1] proc: make /proc/self point to thread Oleg Nesterov
2013-08-27 14:40                                       ` [PATCH 1/1] " Oleg Nesterov
2013-08-27 16:39                                         ` Linus Torvalds
2013-08-27 17:49                                           ` Oleg Nesterov
2013-08-27 18:15                                             ` Linus Torvalds
2013-08-27 18:28                                               ` Oleg Nesterov
     [not found]                                     ` <CALCETrXP-mYBPRon=0NzexW1FK1Qxz2+Bwv7-WeHBQpvW7ywRg@mail.gmail.com>
2013-08-27 15:45                                       ` [PATCH] proc: make proc_fd_permission() thread-friendly Oleg Nesterov
2013-08-26 18:32                                 ` Eric W. Biederman
2013-08-26 18:46                                   ` Oleg Nesterov
2013-08-26 18:56                                     ` Oleg Nesterov
2013-08-26 19:10                                     ` Eric W. Biederman
2013-08-27 14:53                                       ` Oleg Nesterov
2013-08-25 18:32                   ` /proc/pid/fd && anon_inode_fops Linus Torvalds
2013-08-25 19:11                     ` Al Viro
2013-08-25 19:17                     ` Andy Lutomirski
2013-09-03 15:58                     ` Pavel Machek
2013-08-25 15:45                 ` Oleg Nesterov
2013-08-22 19:39       ` [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH) Willy Tarreau

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=7d1419dda1da70a8ad915f85b093a58b86bcaf3b.1377630856.git.luto@amacapital.net \
    --to=luto@amacapital.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=security@kernel.org \
    --cc=spender@grsecurity.net \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=w@1wt.eu \
    /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;
as well as URLs for NNTP newsgroup(s).