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
| ||
|
Date: Fri, 3 Aug 2018 16:18:20 +0800 From: jiangyiwen <jiangyiwen@...wei.com> To: Dominique Martinet <asmadeus@...ewreck.org> CC: Eric Van Hensbergen <ericvh@...il.com>, Ron Minnich <rminnich@...dia.gov>, Latchesar Ionkov <lucho@...kov.net>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, <v9fs-developer@...ts.sourceforge.net>, <netdev@...r.kernel.org> Subject: Re: [V9fs-developer] [PATCH] net/9p: avoid request size exceed to the virtqueue number in the zero copy On 2018/8/3 15:32, Dominique Martinet wrote: > jiangyiwen wrote on Fri, Aug 03, 2018: >> Unfortunately, when the address(input and response headers) are not >> at page boundary, it will need two extra entry in the zero copy, or >> else it will cause sg array out of bounds. >> >> To avoid the problem, we should subtract two pages for maxsize. > > Good catch, that must have been painful to figure. > > Given we know how big the headers are (something like 11 or 23 bytes > depending on the op/direction, it's capped by P9_IOHDRSZ at 24), > couldn't we just cheat and not use the start of the buffer if we detect > it's overlapping? > Actually, generally the P9_IOHDRSZ will not cause the problem, because 24 bytes is too small, but P9_ZC_HDR_SZ(4096 bytes) often cause two pages. So I have a question about why we need to use P9_ZC_HDR_SZ, actually we may use P9_IOHDRSZ instead. > It's probably faster to memcpy a few bytes than to use two pages for the > sg list... > It's definitely ugly though, just taking more margin here is probably > just as good. > > > > I'm going to be picky about English again, sorry, please bear with me. > >> Subject: net/9p: avoid request size exceed to the virtqueue number in >> the zero copy > > This is >72 characters so a little bit too long, if possible to shorten > it. > I'm also not sure 'exceed' is a noun so I probably wouldn't have > understood this sentence without the rest of the message... > > The balance is difficult but it doesn't need to contain too much details > either something like "9p/virtio: reduce transport maxsize" is simple > but probably enough as it describes what is done: someone can look at > the rest of the message for the justification. > Thanks, I will resend the patch later. > > >> Signed-off-by: Yiwen Jiang <jiangyiwen@...wei.com> >> --- >> net/9p/trans_virtio.c | 9 +++++---- >> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c >> index 6265d1d..63591b2 100644 >> --- a/net/9p/trans_virtio.c >> +++ b/net/9p/trans_virtio.c >> @@ -754,11 +754,12 @@ static void p9_virtio_remove(struct virtio_device *vdev) >> .cancel = p9_virtio_cancel, >> /* >> * We leave one entry for input and one entry for response >> - * headers. We also skip one more entry to accomodate, address >> - * that are not at page boundary, that can result in an extra >> - * page in zero copy. >> + * headers. We also skip three more entrys to accomodate > > "entry"'s plural is "entries", this word is in checkpatch's dictionary > as a common typo Thanks, I will modify it. > >> + * (input + response headers + data pages), address >> + * that are not at page boundary, that can result in >> + * an extra page in zero copy. >> */ >> - .maxsize = PAGE_SIZE * (VIRTQUEUE_NUM - 3), >> + .maxsize = PAGE_SIZE * (VIRTQUEUE_NUM - 5), >> .def = 1, >> .owner = THIS_MODULE, >> }; > > Thanks, >
Powered by blists - more mailing lists