linux-kernel - [PATCH 17/17] ecryptfs: use vfs_get

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 12 Sep 2016 21:29:19 +0200
From:   Miklos Szeredi <mszeredi@...hat.com>
To:     linux-fsdevel@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org, Al Viro <viro@...iv.linux.org.uk>,
        Tyler Hicks <tyhicks@...onical.com>
Subject: [PATCH 17/17] ecryptfs: use vfs_get_link()

Here again we are copying form one buffer to another, while jumping through
hoops to make kernel memory look like userspace memory.

For no good reason, since vfs_get_link() provides exactly what is needed.

As a bonus, now the security hook for readlink is also called on the
underlying inode.

Note: this can be called from link-following context.  But this is okay:

 - not in RCU mode

 - e54ad7f1ee26 ("proc: prevent stacking filesystems on top")

 - ecryptfs is *reading* the underlying symlink not following it, so the
   right security hook is being called

Signed-off-by: Miklos Szeredi <mszeredi@...hat.com>
Cc: Tyler Hicks <tyhicks@...onical.com>
---
 fs/ecryptfs/inode.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index fb2d831b7030..95e51b2af2de 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -627,26 +627,23 @@ out_lock:
 
 static char *ecryptfs_readlink_lower(struct dentry *dentry, size_t *bufsiz)
 {
+	DEFINE_DELAYED_CALL(done);
 	struct dentry *lower_dentry = ecryptfs_dentry_to_lower(dentry);
-	char *lower_buf;
+	const char *link;
 	char *buf;
-	mm_segment_t old_fs;
 	int rc;
 
-	lower_buf = kmalloc(PATH_MAX, GFP_KERNEL);
-	if (!lower_buf)
-		return ERR_PTR(-ENOMEM);
-	old_fs = get_fs();
-	set_fs(get_ds());
-	rc = vfs_readlink(lower_dentry, (char __user *)lower_buf, PATH_MAX);
-	set_fs(old_fs);
-	if (rc < 0)
-		goto out;
+	link = vfs_get_link(lower_dentry, d_inode(lower_dentry), &done);
+	if (IS_ERR(link))
+		return ERR_CAST(link);
+
 	rc = ecryptfs_decode_and_decrypt_filename(&buf, bufsiz, dentry->d_sb,
-						  lower_buf, rc);
-out:
-	kfree(lower_buf);
-	return rc ? ERR_PTR(rc) : buf;
+						  link, strlen(link));
+	do_delayed_call(&done);
+	if (rc)
+		return ERR_PTR(rc);
+
+	return buf;
 }
 
 static const char *ecryptfs_get_link(struct dentry *dentry,
-- 
2.5.5