linux-kernel - Re: [PATCH 4/5] 9p: introduce async read requests

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20161210015059.GD1555@ZenIV.linux.org.uk>
Date:   Sat, 10 Dec 2016 01:50:59 +0000
From:   Al Viro <viro@...IV.linux.org.uk>
To:     Stefano Stabellini <sstabellini@...nel.org>
Cc:     v9fs-developer@...ts.sourceforge.net, ericvh@...il.com,
        rminnich@...dia.gov, lucho@...kov.net, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/5] 9p: introduce async read requests

On Thu, Dec 08, 2016 at 12:59:05PM -0800, Stefano Stabellini wrote:


> +		} else {
> +			req = p9_client_get_req(clnt, P9_TREAD, "dqd", fid->fid, offset, rsize);
> +			if (IS_ERR(req)) {
> +				*err = PTR_ERR(req);
> +				break;
> +			}
> +			req->rsize = iov_iter_get_pages_alloc(to, &req->pagevec, 
> +					(size_t)rsize, &req->offset);
> +			req->kiocb = iocb;
> +			for (i = 0; i < req->rsize; i += PAGE_SIZE)
> +				page_cache_get_speculative(req->pagevec[i/PAGE_SIZE]);
> +			req->callback = p9_client_read_complete;
> +
> +			*err = clnt->trans_mod->request(clnt, req);
> +			if (*err < 0) {
> +				clnt->status = Disconnected;
> +				release_pages(req->pagevec,
> +						(req->rsize + PAGE_SIZE - 1) / PAGE_SIZE,
> +						true);
> +				kvfree(req->pagevec);
> +				p9_free_req(clnt, req);
> +				break;
> +			}
> +
> +			*err = -EIOCBQUEUED;

IDGI.  AFAICS, your code will result in shitloads of short reads - every
time when you give it a multi-iovec array, only the first one will be
issued and the rest won't be even looked at.  Sure, it is technically
legal, but I very much doubt that aio users will be happy with that.

What am I missing here?