[PATCH] vfs: avoid recopying filename in getname_flags

[Boqun Feng]

In the current implementation of getname_flags, filename in the
user-space will be recopied if it takes more space that
EMBEDDED_NAME_MAX, however, at this moment, EMBEDDED_NAME_MAX bytes of
the filename are already copied into kernel space, the only reason why
the recopy is needed is that "kname" needs to be relocated.

And the recopy can be avoided if we change the memory layout of the
"names_cache" allocation. By putting the struct "filename" at the tail
of the allocation instead of the head, relocation of kname is avoided.

Once putting the struct at the tail, each byte in the user space will be
copied only one time, so the recopy is avoided and code is more clear.

Of course, other functions aware of the layout of the names_cache
allocation, i.e., getname_kernel and putname also need to be modified to
adapt to the new layout.

This patch is based on v4.0-rc1.

Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Cc: Paul Moore <pmoore@xxxxxxxxxx>
Signed-off-by: Boqun Feng <boqun.feng@xxxxxxxxx>
---
 fs/namei.c | 50 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index c83145a..3be372b 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -133,21 +133,20 @@ getname_flags(const char __user *filename, int flags, int *empty)
 	if (result)
 		return result;
 
-	result = __getname();
-	if (unlikely(!result))
+	kname = __getname();
+	if (unlikely(!kname))
 		return ERR_PTR(-ENOMEM);
-	result->refcnt = 1;
 
 	/*
 	 * First, try to embed the struct filename inside the names_cache
 	 * allocation
 	 */
-	kname = (char *)result + sizeof(*result);
+	result = (struct filename *)(kname + EMBEDDED_NAME_MAX);
 	result->name = kname;
 	result->separate = false;
+	result->refcnt = 1;
 	max = EMBEDDED_NAME_MAX;
 
-recopy:
 	len = strncpy_from_user(kname, filename, max);
 	if (unlikely(len < 0)) {
 		err = ERR_PTR(len);
@@ -157,23 +156,34 @@ recopy:
 	/*
 	 * Uh-oh. We have a name that's approaching PATH_MAX. Allocate a
 	 * separate struct filename so we can dedicate the entire
-	 * names_cache allocation for the pathname, and re-do the copy from
+	 * names_cache allocation for the pathname, and continue the copy from
 	 * userland.
 	 */
-	if (len == EMBEDDED_NAME_MAX && max == EMBEDDED_NAME_MAX) {
-		kname = (char *)result;
-
+	if (len == EMBEDDED_NAME_MAX) {
 		result = kzalloc(sizeof(*result), GFP_KERNEL);
 		if (!result) {
 			err = ERR_PTR(-ENOMEM);
-			result = (struct filename *)kname;
+			result = (struct filename *)(kname + EMBEDDED_NAME_MAX);
 			goto error;
 		}
 		result->name = kname;
 		result->separate = true;
 		result->refcnt = 1;
-		max = PATH_MAX;
-		goto recopy;
+		max = PATH_MAX - EMBEDDED_NAME_MAX;
+		/* we can't simply add the number of rest chars we copy to len,
+		 * since strncpy_from_user may return negative to indicate
+		 * something is wrong, so we do the addition later, after
+		 * strncpy_from_user succeeds, and we know we already copy
+		 * EMBEDDED_NAME_MAX chars.
+		 */
+		len = strncpy_from_user(kname + EMBEDDED_NAME_MAX,
+				filename + EMBEDDED_NAME_MAX, max);
+
+		if (unlikely(len < 0)) {
+			err = ERR_PTR(len);
+			goto error;
+		}
+		len += EMBEDDED_NAME_MAX;
 	}
 
 	/* The empty path is special. */
@@ -209,28 +219,30 @@ struct filename *
 getname_kernel(const char * filename)
 {
 	struct filename *result;
+	char *kname;
 	int len = strlen(filename) + 1;
 
-	result = __getname();
-	if (unlikely(!result))
+	kname = __getname();
+	if (unlikely(!kname))
 		return ERR_PTR(-ENOMEM);
 
 	if (len <= EMBEDDED_NAME_MAX) {
-		result->name = (char *)(result) + sizeof(*result);
+		result = (struct filename *)(kname + EMBEDDED_NAME_MAX);
+		result->name = kname;
 		result->separate = false;
 	} else if (len <= PATH_MAX) {
 		struct filename *tmp;
 
 		tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
 		if (unlikely(!tmp)) {
-			__putname(result);
+			__putname(kname);
 			return ERR_PTR(-ENOMEM);
 		}
-		tmp->name = (char *)result;
+		tmp->name = kname;
 		tmp->separate = true;
 		result = tmp;
 	} else {
-		__putname(result);
+		__putname(kname);
 		return ERR_PTR(-ENAMETOOLONG);
 	}
 	memcpy((char *)result->name, filename, len);
@@ -253,7 +265,7 @@ void putname(struct filename *name)
 		__putname(name->name);
 		kfree(name);
 	} else
-		__putname(name);
+		__putname(name->name);
 }
 
 static int check_acl(struct inode *inode, int mask)
-- 
2.3.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/