From 0749708352fddbe0fa81fc25f96e3b1f77c655f4 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Sat, 28 Apr 2012 14:27:38 -0700 Subject: x86: make word-at-a-time strncpy_from_user clear bytes at the end This makes the newly optimized x86 strncpy_from_user clear the final bytes in the word past the final NUL character, rather than copy them as the word they were in the source. NOTE! Unlike the silly semantics of the libc 'strncpy()' function, the kernel strncpy_from_user() has never cleared all of the end of the destination buffer. And neither does it do so now: it only clears the bytes at the end of the last word it copied. So why make this change at all? It doesn't really cost us anything extra (we have to calculate the mask to get the length anyway), and it means that *if* any user actually cares about zeroing the whole buffer, they can do a "memset()" before the strncpy_from_user(), and we will no longer write random bytes after the NUL character. In particular, the buffer contents will now at no point contain random source data from beyond the end of the string. In other words, it makes behavior a bit more repeatable at no new cost, so it's a small cleanup. I've been carrying this as a patch for the last few weeks or so in my tree (done at the same time the sign error was fixed in commit 12e993b89464), I might as well commit it. Signed-off-by: Linus Torvalds diff --git a/arch/x86/lib/usercopy.c b/arch/x86/lib/usercopy.c index d6ae30b..2e4e4b0 100644 --- a/arch/x86/lib/usercopy.c +++ b/arch/x86/lib/usercopy.c @@ -44,13 +44,6 @@ copy_from_user_nmi(void *to, const void __user *from, unsigned long n) } EXPORT_SYMBOL_GPL(copy_from_user_nmi); -static inline unsigned long count_bytes(unsigned long mask) -{ - mask = (mask - 1) & ~mask; - mask >>= 7; - return count_masked_bytes(mask); -} - /* * Do a strncpy, return length of string without final '\0'. * 'count' is the user-supplied count (return 'count' if we @@ -69,16 +62,19 @@ static inline long do_strncpy_from_user(char *dst, const char __user *src, long max = count; while (max >= sizeof(unsigned long)) { - unsigned long c; + unsigned long c, mask; /* Fall back to byte-at-a-time if we get a page fault */ if (unlikely(__get_user(c,(unsigned long __user *)(src+res)))) break; - /* This can write a few bytes past the NUL character, but that's ok */ + mask = has_zero(c); + if (mask) { + mask = (mask - 1) & ~mask; + mask >>= 7; + *(unsigned long *)(dst+res) = c & mask; + return res + count_masked_bytes(mask); + } *(unsigned long *)(dst+res) = c; - c = has_zero(c); - if (c) - return res + count_bytes(c); res += sizeof(unsigned long); max -= sizeof(unsigned long); } -- cgit v0.10.2 From 3f9f0aa687d45cbc8e7bb3cfd3aab555dcc8872e Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Sat, 28 Apr 2012 14:38:32 -0700 Subject: VFS: clean up and simplify getname_flags() This removes a number of silly games around strncpy_from_user() in do_getname(), and removes that helper function entirely. We instead make getname_flags() just use strncpy_from_user() properly directly. Removing the wrapper function simplifies things noticeably, mostly because we no longer play the unnecessary games with segments (x86 strncpy_from_user() no longer needs the hack), but also because the empty path handling is just much more obvious. The return value of "strncpy_to_user()" is much more obvious than checking an odd error return case from do_getname(). [ non-x86 architectures were notified of this change several weeks ago, since it is possible that they have copied the old broken x86 strncpy_from_user. But nobody reacted, so .. See http://www.spinics.net/lists/linux-arch/msg17313.html for details ] Cc: linux-arch@vger.kernel.org Signed-off-by: Linus Torvalds diff --git a/fs/namei.c b/fs/namei.c index 0062dd1..494b1ca 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -116,47 +116,37 @@ * POSIX.1 2.4: an empty pathname is invalid (ENOENT). * PATH_MAX includes the nul terminator --RR. */ -static int do_getname(const char __user *filename, char *page) -{ - int retval; - unsigned long len = PATH_MAX; - - if (!segment_eq(get_fs(), KERNEL_DS)) { - if ((unsigned long) filename >= TASK_SIZE) - return -EFAULT; - if (TASK_SIZE - (unsigned long) filename < PATH_MAX) - len = TASK_SIZE - (unsigned long) filename; - } - - retval = strncpy_from_user(page, filename, len); - if (retval > 0) { - if (retval < len) - return 0; - return -ENAMETOOLONG; - } else if (!retval) - retval = -ENOENT; - return retval; -} - static char *getname_flags(const char __user *filename, int flags, int *empty) { - char *result = __getname(); - int retval; + char *result = __getname(), *err; + int len; - if (!result) + if (unlikely(!result)) return ERR_PTR(-ENOMEM); - retval = do_getname(filename, result); - if (retval < 0) { - if (retval == -ENOENT && empty) + len = strncpy_from_user(result, filename, PATH_MAX); + err = ERR_PTR(len); + if (unlikely(len < 0)) + goto error; + + /* The empty path is special. */ + if (unlikely(!len)) { + if (empty) *empty = 1; - if (retval != -ENOENT || !(flags & LOOKUP_EMPTY)) { - __putname(result); - return ERR_PTR(retval); - } + err = ERR_PTR(-ENOENT); + if (!(flags & LOOKUP_EMPTY)) + goto error; } - audit_getname(result); - return result; + + err = ERR_PTR(-ENAMETOOLONG); + if (likely(len < PATH_MAX)) { + audit_getname(result); + return result; + } + +error: + __putname(result); + return err; } char *getname(const char __user * filename) -- cgit v0.10.2 From e994defb7b6813ba6fa7a2a36e86d2455ad1dc35 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Sat, 28 Apr 2012 14:55:17 -0700 Subject: VFS: make vfs_fstat() use f[get|put]_light() Use the *_light() versions that properly avoid doing the file user count updates when they are unnecessary. Signed-off-by: Linus Torvalds diff --git a/fs/stat.c b/fs/stat.c index c733dc5..88b36c7 100644 --- a/fs/stat.c +++ b/fs/stat.c @@ -57,12 +57,13 @@ EXPORT_SYMBOL(vfs_getattr); int vfs_fstat(unsigned int fd, struct kstat *stat) { - struct file *f = fget(fd); + int fput_needed; + struct file *f = fget_light(fd, &fput_needed); int error = -EBADF; if (f) { error = vfs_getattr(f->f_path.mnt, f->f_path.dentry, stat); - fput(f); + fput_light(f, fput_needed); } return error; } -- cgit v0.10.2