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

[Chuck Lever]

Andrew Morton wrote:
> Chuck Lever <cel@xxxxxxxxxx> 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/