| 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
| ||
|
Message-Id: <1424867468-17346-1-git-send-email-boqun.feng@gmail.com>
Date: Wed, 25 Feb 2015 20:31:08 +0800
From: Boqun Feng <boqun.feng@...il.com>
To: linux-fsdevel@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, Boqun Feng <boqun.feng@...il.com>,
Al Viro <viro@...iv.linux.org.uk>,
Paul Moore <pmoore@...hat.com>
Subject: [PATCH] vfs: avoid recopying filename in getname_flags
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@...iv.linux.org.uk>
Cc: Paul Moore <pmoore@...hat.com>
Signed-off-by: Boqun Feng <boqun.feng@...il.com>
---
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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists