In commit 9f79b78ef744 ("Convert filldir[64]() from __put_user() to unsafe_put_user()") I made filldir() use unsafe_put_user(), which improves code generation on x86 enormously. But because we didn't have a "unsafe_copy_to_user()", the dirent name copy was also done by hand with unsafe_put_user() in a loop, and it turns out that a lot of other architectures didn't like that, because unlike x86, they have various alignment issues. Most non-x86 architectures trap and fix it up, and some (like xtensa) will just fail unaligned put_user() accesses unconditionally. Which makes that "copy using put_user() in a loop" not work for them at all. I could make that code do explicit alignment etc, but the architectures that don't like unaligned accesses also don't really use the fancy "user_access_begin/end()" model, so they might just use the regular old __copy_to_user() interface. So this commit takes that looping implementation, turns it into the x86 version of "unsafe_copy_to_user()", and makes other architectures implement the unsafe copy version as __copy_to_user() (the same way they do for the other unsafe_xyz() accessor functions). Note that it only does this for the copying _to_ user space, and we still don't have a unsafe version of copy_from_user(). That's partly because we have no current users of it, but also partly because the copy_from_user() case is slightly different and cannot efficiently be implemented in terms of a unsafe_get_user() loop (because gcc can't do asm goto with outputs). It would be trivial to do this using "rep movsb", which would work really nicely on newer x86 cores, but really badly on some older ones. Al Viro is looking at cleaning up all our user copy routines to make this all a non-issue, but for now we have this simple-but-stupid version for x86 that works fine for the dirent name copy case because those names are short strings and we simply don't need anything fancier.
Unified · Split
| @@ -734,5 +734,28 @@ do { \ · 733 unmodified lines | ||
| 734 | 734 | if (unlikely(__gu_err)) goto err_label; \ |
| 735 | 735 | } while (0) |
| 736 | 736 | |
| 737 | +/* | |
| 738 | + * We want the unsafe accessors to always be inlined and use | |
| 739 | + * the error labels - thus the macro games. | |
| 740 | + */ | |
| 741 | +#define unsafe_copy_loop(dst, src, len, type, label) \ | |
| 742 | + while (len >= sizeof(type)) { \ | |
| 743 | + unsafe_put_user(*(type *)src,(type __user *)dst,label); \ | |
| 744 | + dst += sizeof(type); \ | |
| 745 | + src += sizeof(type); \ | |
| 746 | + len -= sizeof(type); \ | |
| 747 | + } | |
| 748 | + | |
| 749 | +#define unsafe_copy_to_user(_dst,_src,_len,label) \ | |
| 750 | +do { \ | |
| 751 | + char __user *__ucu_dst = (_dst); \ | |
| 752 | + const char *__ucu_src = (_src); \ | |
| 753 | + size_t __ucu_len = (_len); \ | |
| 754 | + unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u64, label); \ | |
| 755 | + unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u32, label); \ | |
| 756 | + unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u16, label); \ | |
| 757 | + unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u8, label); \ | |
| 758 | +} while (0) | |
| 759 | + | |
| 737 | 760 | #endif /* _ASM_X86_UACCESS_H */ |
| 738 | 761 | |
| @@ -27,53 +27,13 @@ · 26 unmodified lines | ||
| 27 | 27 | /* |
| 28 | 28 | * Note the "unsafe_put_user() semantics: we goto a |
| 29 | 29 | * label for errors. |
| 30 | − * | |
| 31 | − * Also note how we use a "while()" loop here, even though | |
| 32 | − * only the biggest size needs to loop. The compiler (well, | |
| 33 | − * at least gcc) is smart enough to turn the smaller sizes | |
| 34 | − * into just if-statements, and this way we don't need to | |
| 35 | − * care whether 'u64' or 'u32' is the biggest size. | |
| 36 | − */ | |
| 37 | −#define unsafe_copy_loop(dst, src, len, type, label) \ | |
| 38 | − while (len >= sizeof(type)) { \ | |
| 39 | − unsafe_put_user(get_unaligned((type *)src), \ | |
| 40 | − (type __user *)dst, label); \ | |
| 41 | − dst += sizeof(type); \ | |
| 42 | − src += sizeof(type); \ | |
| 43 | − len -= sizeof(type); \ | |
| 44 | − } | |
| 45 | − | |
| 46 | −/* | |
| 47 | − * We avoid doing 64-bit copies on 32-bit architectures. They | |
| 48 | − * might be better, but the component names are mostly small, | |
| 49 | − * and the 64-bit cases can end up being much more complex and | |
| 50 | − * put much more register pressure on the code, so it's likely | |
| 51 | − * not worth the pain of unaligned accesses etc. | |
| 52 | − * | |
| 53 | − * So limit the copies to "unsigned long" size. I did verify | |
| 54 | − * that at least the x86-32 case is ok without this limiting, | |
| 55 | − * but I worry about random other legacy 32-bit cases that | |
| 56 | − * might not do as well. | |
| 57 | − */ | |
| 58 | −#define unsafe_copy_type(dst, src, len, type, label) do { \ | |
| 59 | − if (sizeof(type) <= sizeof(unsigned long)) \ | |
| 60 | − unsafe_copy_loop(dst, src, len, type, label); \ | |
| 61 | −} while (0) | |
| 62 | − | |
| 63 | −/* | |
| 64 | − * Copy the dirent name to user space, and NUL-terminate | |
| 65 | − * it. This should not be a function call, since we're doing | |
| 66 | − * the copy inside a "user_access_begin/end()" section. | |
| 67 | 30 | */ |
| 68 | 31 | #define unsafe_copy_dirent_name(_dst, _src, _len, label) do { \ |
| 69 | 32 | char __user *dst = (_dst); \ |
| 70 | 33 | const char *src = (_src); \ |
| 71 | 34 | size_t len = (_len); \ |
| 72 | − unsafe_copy_type(dst, src, len, u64, label); \ | |
| 73 | − unsafe_copy_type(dst, src, len, u32, label); \ | |
| 74 | − unsafe_copy_type(dst, src, len, u16, label); \ | |
| 75 | − unsafe_copy_type(dst, src, len, u8, label); \ | |
| 76 | − unsafe_put_user(0, dst, label); \ | |
| 35 | + unsafe_put_user(0, dst+len, label); \ | |
| 36 | + unsafe_copy_to_user(dst, src, len, label); \ | |
| 77 | 37 | } while (0) |
| 78 | 38 | |
| 79 | 39 | |
| @@ -355,8 +355,10 @@ extern long strnlen_unsafe_user(const void __user *unsafe_addr, long count); · 354 unmodified lines | ||
| 355 | 355 | #ifndef user_access_begin |
| 356 | 356 | #define user_access_begin(ptr,len) access_ok(ptr, len) |
| 357 | 357 | #define user_access_end() do { } while (0) |
| 358 | −#define unsafe_get_user(x, ptr, err) do { if (unlikely(__get_user(x, ptr))) goto err; } while (0) | |
| 359 | −#define unsafe_put_user(x, ptr, err) do { if (unlikely(__put_user(x, ptr))) goto err; } while (0) | |
| 358 | +#define unsafe_op_wrap(op, err) do { if (unlikely(op)) goto err; } while (0) | |
| 359 | +#define unsafe_get_user(x,p,e) unsafe_op_wrap(__get_user(x,p),e) | |
| 360 | +#define unsafe_put_user(x,p,e) unsafe_op_wrap(__put_user(x,p),e) | |
| 361 | +#define unsafe_copy_to_user(d,s,l,e) unsafe_op_wrap(__copy_to_user(d,s,l),e) | |
| 360 | 362 | static inline unsigned long user_access_save(void) { return 0UL; } |
| 361 | 363 | static inline void user_access_restore(unsigned long flags) { } |
| 362 | 364 | #endif |