lkml.org 
[lkml]   [2014]   [Oct]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v11 06/21] vfs: Add copy_to_iter(), copy_from_iter() and iov_iter_zero()
On Thu, Oct 16, 2014 at 03:33:55PM +0200, Mathieu Desnoyers wrote:
> > +static size_t copy_to_iter_iovec(void *from, size_t bytes, struct iov_iter *i)
> > +{
[...]
> > + left = __copy_to_user(buf, from, copy);
>
> How comes this function uses __copy_to_user without any access_ok()
> check ? This has security implications.

The access_ok() check is done higher up the call-chain if it's appropriate.
These functions can be (intentionally) called to access kernel addresses,
so it wouldn't be appropriate to do that here.

> > +static size_t copy_page_to_iter_bvec(struct page *page, size_t offset,
> > + size_t bytes, struct iov_iter *i)
> > +{
> > + void *kaddr = kmap_atomic(page);
> > + size_t wanted = copy_to_iter_bvec(kaddr + offset, bytes, i);
>
> missing newline.
>
> > + kunmap_atomic(kaddr);
> > + return wanted;
> > +}

Are you seriously suggesting that:

static size_t copy_page_to_iter_bvec(struct page *page, size_t offset,
size_t bytes, struct iov_iter *i)
{
void *kaddr = kmap_atomic(page);
size_t wanted = copy_to_iter_bvec(kaddr + offset, bytes, i);

kunmap_atomic(kaddr);
return wanted;
}

is more readable than without the newline? I can see the point of the
rule for functions with a lot of variables, or a lot of lines, but I
don't see the point of it for such a small function.

In any case, this patch is now upstream, so I shan't be proposing any
stylistic changes for it.


\
 
 \ /
  Last update: 2014-10-16 16:41    [W:0.116 / U:0.400 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site