On Thu, Mar 5, 2020 at 10:03 AM Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx> wrote: > On 02/03/2020 19.31, Jann Horn wrote: > > On Mon, Mar 2, 2020 at 7:17 PM Alexander Potapenko <glider@xxxxxxxxxx> wrote: > >> On Mon, Mar 2, 2020 at 3:00 PM Joe Perches <joe@xxxxxxxxxxx> wrote: > >>> > >>> So? CONFIG_INIT_STACK_ALL by design slows down code. > >> Correct. > >> > >>> This marking would likely need to be done for nearly all > >>> 3000+ copy_from_user entries. > >> Unfortunately, yes. I was just hoping to do so for a handful of hot > >> cases that we encounter, but in the long-term a compiler solution must > >> supersede them. > >> > >>> Why not try to get something done on the compiler side > >>> to mark the function itself rather than the uses? > >> This is being worked on in the meantime as well (see > >> http://lists.llvm.org/pipermail/cfe-dev/2020-February/064633.html) > >> Do you have any particular requisitions about how this should look on > >> the source level? > > > > Just thinking out loud: Should this be a function attribute, or should > > it be a builtin - something like __builtin_assume_initialized(ptr, > > len)? That would make it also work for macros, > > But with macros (and static inlines), the compiler sees all the > initialization being done, no? Depends on how the macro writes to the buffer, whether it's a normal write or happens through another function call or whatever. > and it might simplify > > the handling of inlining in the compiler. And you wouldn't need such a > > complicated attribute that refers to function arguments by index and > > such. > > Does copy_from_user guarantee to zero-initialize the remaining buffer if > copying fails partway through? Basically yes. From include/linux/uaccess.h: static __always_inline unsigned long __must_check copy_from_user(void *to, const void __user *from, unsigned long n) { if (likely(check_copy_size(to, n, false))) n = _copy_from_user(to, from, n); return n; } check_copy_size() should be optimized out entirely for straightforward use of stack objects; it will only return false if the specified address range crosses beyond an allocation boundary. _copy_from_user() is defined as follows (there are two possible definitions, both of them have the same method body, but they differ in whether the function is inline - which one is used depends on the architecture): static inline __must_check unsigned long _copy_from_user(void *to, const void __user *from, unsigned long n) { unsigned long res = n; might_fault(); if (likely(access_ok(from, n))) { kasan_check_write(to, n); res = raw_copy_from_user(to, from, n); } if (unlikely(res)) memset(to + (n - res), 0, res); return res; } So annotating _copy_from_user(), or calling a magic fake-initialization builtin directly before calling _copy_from_user(), should be safe. As long as the compiler can eliminate the call to check_copy_size(), that should then make that propagate up to the caller of copy_from_user(). (You could also try to annotate copy_from_user() directly, but I'm not sure whether doing it before the bounds check might confuse the compiler somehow.) _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
