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-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html