On 05/09/2019 13.05, Christian Brauner wrote: > On Thu, Sep 05, 2019 at 06:19:22AM +1000, Aleksa Sarai wrote: >> + if (unlikely(!access_ok(dst, usize))) >> + return -EFAULT; >> + >> + /* Deal with trailing bytes. */ >> + if (usize < ksize) { >> + if (memchr_inv(src + size, 0, rest)) >> + return -EFBIG; >> + } else if (usize > ksize) { >> + if (__memzero_user(dst + size, rest)) >> + return -EFAULT; > > Is zeroing that memory really our job? Seems to me we should just check > it is zeroed. Of course it is, otherwise you'd require userspace to clear the output buffer it gives us, which in the majority of cases is wasted work. It's much easier to reason about if we just say "the kernel populates [uaddr, uaddr + usize)". It's completely symmetric to copy_struct_from_user doing a memset() of the tail of the kernel buffer in case of ksize>usize - you wouldn't want to require the kernel callers to pass a zeroed buffer to copy_struct_from_user() - it's just that when we memset(__user*), there's an error check to do. Rasmus