| 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
| ||
|
Date: Thu, 26 Mar 2015 02:45:52 +0800
From: Boqun Feng <boqun.feng@...il.com>
To: linux-fsdevel@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, Al Viro <viro@...iv.linux.org.uk>,
Paul Moore <pmoore@...hat.com>,
Boqun Feng <boqun.feng@...il.com>
Subject: [PATCH v2] vfs: avoid recopying file names in getname_flags
In the current implementation of getname_flags, a file name 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 file name are already copied into kernel address space, the only
reason why the recopy is needed is that "kname", which points to the
string of the file name, needs to be relocated.
And the recopy can be avoided if we change the memory layout of the
`names_cachep` allocation. By putting `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.
Of course, other functions aware of the layout of the `names_cachep`
allocation, i.e., getname_kernel and putname also need to be modified to
adapt to the new layout.
Since we change the layout of `names_cachep` allocation, in order to
figure out whether kname and the struct are separate, we now need to
check whether the file name string starts at the address
EMBEDDED_NAME_MAX bytes before the address of the struct. As a result,
`iname`, which points the end of `struct filename`, is not needed
anymore.
Signed-off-by: Boqun Feng <boqun.feng@...il.com>
---
v1 --> v2:
Rebase the patch onto the for-next branch of Al's vfs repo.
fs/namei.c | 45 ++++++++++++++++++++++++++++-----------------
include/linux/fs.h | 1 -
2 files changed, 28 insertions(+), 18 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 7a11ec1..6d04029 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -119,7 +119,7 @@
* PATH_MAX includes the nul terminator --RR.
*/
-#define EMBEDDED_NAME_MAX (PATH_MAX - offsetof(struct filename, iname))
+#define EMBEDDED_NAME_MAX (PATH_MAX - sizeof(struct filename))
struct filename *
getname_flags(const char __user *filename, int flags, int *empty)
@@ -132,44 +132,53 @@ 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);
/*
* First, try to embed the struct filename inside the names_cache
* allocation
*/
- kname = (char *)result->iname;
+ result = (struct filename *)(kname + EMBEDDED_NAME_MAX);
result->name = kname;
len = strncpy_from_user(kname, filename, EMBEDDED_NAME_MAX);
if (unlikely(len < 0)) {
- __putname(result);
+ __putname(kname);
return ERR_PTR(len);
}
/*
* 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 (unlikely(len == EMBEDDED_NAME_MAX)) {
- kname = (char *)result;
-
result = kzalloc(sizeof(*result), GFP_KERNEL);
if (unlikely(!result)) {
__putname(kname);
return ERR_PTR(-ENOMEM);
}
result->name = kname;
- len = strncpy_from_user(kname, filename, PATH_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,
+ PATH_MAX - EMBEDDED_NAME_MAX);
+
if (unlikely(len < 0)) {
__putname(kname);
kfree(result);
return ERR_PTR(len);
}
+
+ len += EMBEDDED_NAME_MAX;
if (unlikely(len == PATH_MAX)) {
__putname(kname);
kfree(result);
@@ -204,26 +213,28 @@ 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->iname;
+ result = (struct filename *)(kname + EMBEDDED_NAME_MAX);
+ result->name = kname;
} 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;
result = tmp;
} else {
- __putname(result);
+ __putname(kname);
return ERR_PTR(-ENAMETOOLONG);
}
memcpy((char *)result->name, filename, len);
@@ -242,11 +253,11 @@ void putname(struct filename *name)
if (--name->refcnt > 0)
return;
- if (name->name != name->iname) {
+ if (name->name != ((char *)name - EMBEDDED_NAME_MAX)) {
__putname(name->name);
kfree(name);
} else
- __putname(name);
+ __putname(name->name);
}
static int check_acl(struct inode *inode, int mask)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index dfbd88a..dd67284 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2166,7 +2166,6 @@ struct filename {
const __user char *uptr; /* original userland pointer */
struct audit_names *aname;
int refcnt;
- const char iname[];
};
extern long vfs_truncate(struct path *, loff_t);
--
2.3.3
--
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