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
| ||
|
Message-ID: <20150211040113.GC29656@ZenIV.linux.org.uk> Date: Wed, 11 Feb 2015 04:01:13 +0000 From: Al Viro <viro@...IV.linux.org.uk> To: Linus Torvalds <torvalds@...ux-foundation.org> Cc: David Miller <davem@...emloft.net>, Andrew Morton <akpm@...ux-foundation.org>, Network Development <netdev@...r.kernel.org>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org> Subject: Re: [GIT] Networking On Tue, Feb 10, 2015 at 06:01:32PM -0800, Linus Torvalds wrote: > On Tue, Feb 10, 2015 at 5:45 PM, Al Viro <viro@...iv.linux.org.uk> wrote: > > > > Could you check if > > > > diff --git a/crypto/af_alg.c b/crypto/af_alg.c > > index eb78fe8..5b11d64 100644 > > --- a/crypto/af_alg.c > > +++ b/crypto/af_alg.c > > @@ -348,7 +348,7 @@ int af_alg_make_sg(struct af_alg_sgl *sgl, struct iov_iter *iter, int len) > > if (n < 0) > > return n; > > > > - npages = PAGE_ALIGN(off + n); > > + npages = DIV_ROUND_UP(off + n, PAGE_SIZE); > > if (WARN_ON(npages == 0)) > > return -EINVAL; > > > > on top of what went into Dave's tree is enough to fix what you are seeing? > > So quite frankly, considering that we already know that 'used' was > also calculated wrong (it stays at zero) and that the one-liner above > is not sufficient on its own, by now I'd like somebody with an actual > test-case to check this thing out., and send me a proper tested patch. > That somebody preferably being you. > > Are there no tests for that crypto interface? I hoped LTP would have them, but it doesn't ;-/ The best I'd been able to find had been in libkcapi, modulo bunch of let result=($result + 1) (in a bash script; replacing with result=$(($result+1)) has dealt with that) and algif-aead module it supplies and wants to test. It reproduces the oopsen just fine and patch below seems to fix things. > Also, I'm unhappy that you made these unrelated and broken changes at > all. Both wrt 'used' and this 'npages' thing, the original code wasn't > wrong, and the pointless changes to correct code not only broke > things, but had nothing to do with the iovec conversion that was the > stated reason for the commit. I've fucked up on those, no arguments, but they *were* very much related to the conversion in question - original code in skcipher_recvmsg() was a loop over iovec segments, with inner loop over each segment. With iov_iter conversion we need a flat loop, and I'd screwed up while massaging the code into that form. And af_alg_make_sg() change had been a consequence of the same - we used to feed it the next subset of the range covered by ->msg_iter.iov[...] in the inner loop there. npages thing is _not_ a replacement of (similar) line you've picked out of that diff - it replaced npages = err; after get_user_pages_fast(). Calling conventions of iov_iter_get_pages() differ - it returns offset of the beginning within the first page (via *start) and amount of data it picked from iterator (as return value). > Grr. I really hate it when I find bugs during the merge window. My > test environment is *not* odd. If _I_ end up being the one finding the > bugs, that means that things must have gotten basically no testing at > all. Out of curiosity - what has triggered that oops on your userland? I certainly _did_ the "allmodconfig and see if it boots" on those; in my usual test image (jessie-amd64 in KVM) nothing has stepped into that one. Again, I'm not trying to excuse the fuckup - just wondering what to add to tests... The patch below appears to fix the problems on the reproducer I'd been able to find. Signed-off-by: Al Viro <viro@...iv.linux.org.uk> --- diff --git a/crypto/af_alg.c b/crypto/af_alg.c index eb78fe8..5b11d64 100644 --- a/crypto/af_alg.c +++ b/crypto/af_alg.c @@ -348,7 +348,7 @@ int af_alg_make_sg(struct af_alg_sgl *sgl, struct iov_iter *iter, int len) if (n < 0) return n; - npages = PAGE_ALIGN(off + n); + npages = DIV_ROUND_UP(off + n, PAGE_SIZE); if (WARN_ON(npages == 0)) return -EINVAL; diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c index 37110fd..0eb31a6 100644 --- a/crypto/algif_skcipher.c +++ b/crypto/algif_skcipher.c @@ -427,11 +427,11 @@ static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock, struct skcipher_sg_list *sgl; struct scatterlist *sg; int err = -EAGAIN; - int used; long copied = 0; lock_sock(sk); while (iov_iter_count(&msg->msg_iter)) { + int used; sgl = list_first_entry(&ctx->tsgl, struct skcipher_sg_list, list); sg = sgl->sg; @@ -439,14 +439,13 @@ static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock, while (!sg->length) sg++; - used = ctx->used; - if (!used) { + if (!ctx->used) { err = skcipher_wait_for_data(sk, flags); if (err) goto unlock; } - used = min_t(unsigned long, used, iov_iter_count(&msg->msg_iter)); + used = min_t(unsigned long, ctx->used, iov_iter_count(&msg->msg_iter)); used = af_alg_make_sg(&ctx->rsgl, &msg->msg_iter, used); err = used; -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists