On 5/19/22 2:05 PM, Jason A. Donenfeld wrote:
> Hi Jens,
>
> On Thu, May 19, 2022 at 01:31:31PM -0600, Jens Axboe wrote:
>> Hi,
>>
>> We recently had a failure on a kernel upgrade because splice no longer
>> works on random/urandom. This is due to:
>>
>> 6e2c7421f02 ("fs: don't allow splice read/write without explicit ops")
>
> Thanks for this. I'd noticed this a few months ago and assumed it has
> just always been that way, and hadn't gotten to looking at what was up.
>
> I'll take a look at these patches in detail when I'm home in a few
> hours, but one thing maybe you can answer more easily than my digging
> is:
Sounds good, thanks!
> There's a lot of attention in random.c devoted to not leaving any output
> around on the stack or in stray buffers. The explicit use of
> copy_to_user() makes it clear that the output isn't being copied
> anywhere other than what's the user's responsibility to cleanup. I'm
> wondering if the switch to copy_to_iter() introduces any buffering or
> gotchas that you might be aware of.
No, it's just a wrapper around copying to the user memory pointed to by
the iov_iter. No extra buffering or anything like that. So I think it
should be fine in that respect, and it actually cleans up the code a bit
imho since the copy_to_iter() since the return value of "bytes copied"
is easier to work with than the "bytes not copied".
> Also you may need to rebase this on the random.git tree at
> https://git.kernel.org/pub/scm/linux/kernel/git/crng/random.git
OK, I will rebase it on that branch, not a problem.
--
Jens Axboe
Hi Jens,
On Thu, May 19, 2022 at 02:49:13PM -0600, Jens Axboe wrote:
> > There's a lot of attention in random.c devoted to not leaving any output
> > around on the stack or in stray buffers. The explicit use of
> > copy_to_user() makes it clear that the output isn't being copied
> > anywhere other than what's the user's responsibility to cleanup. I'm
> > wondering if the switch to copy_to_iter() introduces any buffering or
> > gotchas that you might be aware of.
>
> No, it's just a wrapper around copying to the user memory pointed to by
> the iov_iter. No extra buffering or anything like that. So I think it
> should be fine in that respect, and it actually cleans up the code a bit
> imho since the copy_to_iter() since the return value of "bytes copied"
> is easier to work with than the "bytes not copied".
Alright, that's good to hear. So even for kernel->kernel writes, the
argument is that what ever buffers are used in the process are the same
ones that the user would be hitting anyway by calling write() on the
destination if this roundtripped through userspace, so nothing changes?
Jason
On 5/19/22 5:13 PM, Jason A. Donenfeld wrote:
> Hi Jens,
>
> On Thu, May 19, 2022 at 02:49:13PM -0600, Jens Axboe wrote:
>>> There's a lot of attention in random.c devoted to not leaving any output
>>> around on the stack or in stray buffers. The explicit use of
>>> copy_to_user() makes it clear that the output isn't being copied
>>> anywhere other than what's the user's responsibility to cleanup. I'm
>>> wondering if the switch to copy_to_iter() introduces any buffering or
>>> gotchas that you might be aware of.
>>
>> No, it's just a wrapper around copying to the user memory pointed to by
>> the iov_iter. No extra buffering or anything like that. So I think it
>> should be fine in that respect, and it actually cleans up the code a bit
>> imho since the copy_to_iter() since the return value of "bytes copied"
>> is easier to work with than the "bytes not copied".
>
> Alright, that's good to hear. So even for kernel->kernel writes, the
> argument is that what ever buffers are used in the process are the same
> ones that the user would be hitting anyway by calling write() on the
> destination if this roundtripped through userspace, so nothing changes?
The source and destination for the copies are exactly the same with the
change as before, so no changes there. The non-user copy is a different
helper.
--
Jens Axboe
Hi Jens,
On Fri, May 20, 2022 at 1:19 AM Jens Axboe <[email protected]> wrote:
> The source and destination for the copies are exactly the same with the
> change as before, so no changes there. The non-user copy is a different
> helper.
Oh, okay. Maybe a silly question but: should we wire up that helper
too? (If I'm understanding your meaning right.) Seems like it'd be a
good idea to just wire up all the things while we're at it.
Jason