Re: [PATCH 5/6] nfs: check all iov segments for correct memory access rights

[Chuck Lever <cel@citi.umich.edu>]

Andrew Morton wrote:
> Chuck Lever <cel@netapp.com> wrote:
>> +static ssize_t check_access_ok(int type, const struct iovec *iov, unsigned long nr_segs)
>> +{
>> +	ssize_t count = 0;
>> +	ssize_t retval = -EINVAL;
>> +	unsigned long seg;
>> +
>> +	for (seg = 0; seg < nr_segs; seg++) {
>> +		void __user *buf = iov[seg].iov_base;
>> +		ssize_t len = (ssize_t) iov[seg].iov_len;
>> +
>> +		if (len < 0)		/* size_t not fitting an ssize_t .. */
>> +			goto out;
> 
> do_readv_writev() already checked for negative iov_len, and that's the more
> appropriate place to do it, rather than duplicating it in each filesystem
> (or forgetting to!)
> 
> So is this check really needed?

Ok, didn't see that function before.  Badari, is this checking still needed?

>> +		if (unlikely(!access_ok(type, buf, len))) {
>> +			retval = -EFAULT;
>> +			goto out;
>> +		}
> 
> Now what's up here?  Why does NFS, at this level, care about the page's
> virtual address?  get_user_pages() will handle that?

Interesting point.  That may be a historical artifact that can be 
removed.  I'll take a look.

-- 
corporate:	cel at netapp dot com
personal:	chucklever at bigfoot dot com