net/9p abuses iov_iter primitives - attempts to copy _from_

a destination-only iov_iter when it handles Rerror arriving in reply to
 zero-copy request.  Not hard to fix, fortunately; it's a prereq for the
 iov_iter_get_pages() work in the second part of iov_iter series,
 ended up in a separate branch.
 
 Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
 -----BEGIN PGP SIGNATURE-----
 
 iHUEABYIAB0WIQQqUNBr3gm4hGXdBJlZ7Krx/gZQ6wUCYurQLQAKCRBZ7Krx/gZQ
 65AiAP9Mmpu3yMWmfMEnTEjBv4iSuG37JdgHE/IE/P6q99opfQEAxThED/nJVuaG
 YZuNUx60OT9Au1hSdfl7EjAN4dg/Kw8=
 =tL2V
 -----END PGP SIGNATURE-----

Merge tag 'pull-work.9p' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs

Pull 9p iov_iter fix from Al Viro:
 "net/9p abuses iov_iter primitives - it attempts to copy _from_ a
  destination-only iov_iter when it handles Rerror arriving in reply to
  zero-copy request.   Not hard to fix, fortunately.

  This is a prereq for the iov_iter_get_pages() work in the second part
  of iov_iter series, ended up in a separate branch"

* tag 'pull-work.9p' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs:
  9p: handling Rerror without copy_from_iter_full()
This commit is contained in:
Linus Torvalds 2022-08-03 14:03:51 -07:00
commit ff89dd08c0
2 changed files with 35 additions and 85 deletions

View file

@ -550,90 +550,6 @@ static int p9_check_errors(struct p9_client *c, struct p9_req_t *req)
return err;
}
/**
* p9_check_zc_errors - check 9p packet for error return and process it
* @c: current client instance
* @req: request to parse and check for error conditions
* @uidata: external buffer containing error
* @in_hdrlen: Size of response protocol buffer.
*
* returns error code if one is discovered, otherwise returns 0
*
* this will have to be more complicated if we have multiple
* error packet types
*/
static int p9_check_zc_errors(struct p9_client *c, struct p9_req_t *req,
struct iov_iter *uidata, int in_hdrlen)
{
int err;
int ecode;
s8 type;
char *ename = NULL;
err = p9_parse_header(&req->rc, NULL, &type, NULL, 0);
/* dump the response from server
* This should be after parse_header which poplulate pdu_fcall.
*/
trace_9p_protocol_dump(c, &req->rc);
if (err) {
p9_debug(P9_DEBUG_ERROR, "couldn't parse header %d\n", err);
return err;
}
if (type != P9_RERROR && type != P9_RLERROR)
return 0;
if (!p9_is_proto_dotl(c)) {
/* Error is reported in string format */
int len;
/* 7 = header size for RERROR; */
int inline_len = in_hdrlen - 7;
len = req->rc.size - req->rc.offset;
if (len > (P9_ZC_HDR_SZ - 7)) {
err = -EFAULT;
goto out_err;
}
ename = &req->rc.sdata[req->rc.offset];
if (len > inline_len) {
/* We have error in external buffer */
if (!copy_from_iter_full(ename + inline_len,
len - inline_len, uidata)) {
err = -EFAULT;
goto out_err;
}
}
ename = NULL;
err = p9pdu_readf(&req->rc, c->proto_version, "s?d",
&ename, &ecode);
if (err)
goto out_err;
if (p9_is_proto_dotu(c) && ecode < 512)
err = -ecode;
if (!err) {
err = p9_errstr2errno(ename, strlen(ename));
p9_debug(P9_DEBUG_9P, "<<< RERROR (%d) %s\n",
-ecode, ename);
}
kfree(ename);
} else {
err = p9pdu_readf(&req->rc, c->proto_version, "d", &ecode);
err = -ecode;
p9_debug(P9_DEBUG_9P, "<<< RLERROR (%d)\n", -ecode);
}
return err;
out_err:
p9_debug(P9_DEBUG_ERROR, "couldn't parse error%d\n", err);
return err;
}
static struct p9_req_t *
p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...);
@ -874,7 +790,7 @@ static struct p9_req_t *p9_client_zc_rpc(struct p9_client *c, int8_t type,
if (err < 0)
goto reterr;
err = p9_check_zc_errors(c, req, uidata, in_hdrlen);
err = p9_check_errors(c, req);
trace_9p_client_res(c, type, req->rc.tag, err);
if (!err)
return req;

View file

@ -377,6 +377,35 @@ static int p9_get_mapped_pages(struct virtio_chan *chan,
}
}
static void handle_rerror(struct p9_req_t *req, int in_hdr_len,
size_t offs, struct page **pages)
{
unsigned size, n;
void *to = req->rc.sdata + in_hdr_len;
// Fits entirely into the static data? Nothing to do.
if (req->rc.size < in_hdr_len)
return;
// Really long error message? Tough, truncate the reply. Might get
// rejected (we can't be arsed to adjust the size encoded in header,
// or string size for that matter), but it wouldn't be anything valid
// anyway.
if (unlikely(req->rc.size > P9_ZC_HDR_SZ))
req->rc.size = P9_ZC_HDR_SZ;
// data won't span more than two pages
size = req->rc.size - in_hdr_len;
n = PAGE_SIZE - offs;
if (size > n) {
memcpy_from_page(to, *pages++, offs, n);
offs = 0;
to += n;
size -= n;
}
memcpy_from_page(to, *pages, offs, size);
}
/**
* p9_virtio_zc_request - issue a zero copy request
* @client: client instance issuing the request
@ -503,6 +532,11 @@ p9_virtio_zc_request(struct p9_client *client, struct p9_req_t *req,
kicked = 1;
p9_debug(P9_DEBUG_TRANS, "virtio request kicked\n");
err = wait_event_killable(req->wq, req->status >= REQ_STATUS_RCVD);
// RERROR needs reply (== error string) in static data
if (req->status == REQ_STATUS_RCVD &&
unlikely(req->rc.sdata[4] == P9_RERROR))
handle_rerror(req, in_hdr_len, offs, in_pages);
/*
* Non kernel buffers are pinned, unpin them
*/