| Message ID | 20200626062948.GA25285@gondor.apana.org.au |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | crypto: af_alg - Fix regression on empty requests | expand |
On Fri, Jun 26, 2020 at 04:29:48PM +1000, Herbert Xu wrote: > > Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com> > Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org> > Fixes: f3c802a1f300 ("crypto: algif_aead - Only wake up when...") > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> Reported-by: kernel test robot <rong.a.chen@intel.com>
On Sat, 27 Jun 2020 at 14:02, Herbert Xu <herbert@gondor.apana.org.au> wrote: > > On Fri, Jun 26, 2020 at 04:29:48PM +1000, Herbert Xu wrote: > > > > Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com> > > Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org> > > Fixes: f3c802a1f300 ("crypto: algif_aead - Only wake up when...") > > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> > > Reported-by: kernel test robot <rong.a.chen@intel.com> Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org> I have applied your patch and tested on Linux-next on arm, x86_64 and i386. All LTP crypto tests got passed. --- patch -p1 < crypto-af_alg---Fix-regression-on-empty-requests.diff patching file crypto/af_alg.c patching file crypto/algif_aead.c patching file crypto/algif_skcipher.c patching file include/crypto/if_alg.h Test output: af_alg02.c:33: PASS: Successfully \"encrypted\" an empty message af_alg05.c:40: PASS: read() expectedly failed with EINVAL Test results summary: af_alg01: pass af_alg02: pass af_alg03: pass af_alg04: pass af_alg05: pass af_alg06: pass ref: https://lkft.validation.linaro.org/scheduler/job/1532020#L1413 https://lkft.validation.linaro.org/scheduler/job/1532019#L1516 https://lkft.validation.linaro.org/scheduler/job/1532026#L935
On Fri, 26 Jun 2020 at 12:00, Herbert Xu <herbert@gondor.apana.org.au> wrote: > > On Tue, Jun 23, 2020 at 10:02:17AM -0700, Eric Biggers wrote: > > > > The source code for the two failing AF_ALG tests is here: > > > > https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/crypto/af_alg02.c > > https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/crypto/af_alg05.c > > > > They use read() and write(), not send() and recv(). > > > > af_alg02 uses read() to read from a "salsa20" request socket without writing > > anything to it. It is expected that this returns 0, i.e. that behaves like > > encrypting an empty message. Since we are on this subject, LTP af_alg02 test case fails on stable 4.9 and stable 4.4 This is not a regression because the test case has been failing from the beginning. Is this test case expected to fail on stable 4.9 and 4.4 ? or any chance to fix this on these older branches ? Test output: af_alg02.c:52: BROK: Timed out while reading from request socket. ref: https://qa-reports.linaro.org/lkft/linux-stable-rc-4.9-oe/build/v4.9.228-191-g082e807235d7/testrun/2884917/suite/ltp-crypto-tests/test/af_alg02/history/ https://qa-reports.linaro.org/lkft/linux-stable-rc-4.9-oe/build/v4.9.228-191-g082e807235d7/testrun/2884606/suite/ltp-crypto-tests/test/af_alg02/log - Naresh
diff --git a/crypto/af_alg.c b/crypto/af_alg.c index 9fcb91ea10c4..2d391117c020 100644 --- a/crypto/af_alg.c +++ b/crypto/af_alg.c @@ -635,7 +635,6 @@ void af_alg_pull_tsgl(struct sock *sk, size_t used, struct scatterlist *dst, if (!ctx->used) ctx->merge = 0; - ctx->init = ctx->more; } EXPORT_SYMBOL_GPL(af_alg_pull_tsgl); @@ -757,8 +756,7 @@ int af_alg_wait_for_data(struct sock *sk, unsigned flags, unsigned min) break; timeout = MAX_SCHEDULE_TIMEOUT; if (sk_wait_event(sk, &timeout, - ctx->init && (!ctx->more || - (min && ctx->used >= min)), + !ctx->more || (min && ctx->used >= min), &wait)) { err = 0; break; @@ -847,7 +845,7 @@ int af_alg_sendmsg(struct socket *sock, struct msghdr *msg, size_t size, } lock_sock(sk); - if (ctx->init && (init || !ctx->more)) { + if (!ctx->more && ctx->used) { err = -EINVAL; goto unlock; } @@ -858,7 +856,6 @@ int af_alg_sendmsg(struct socket *sock, struct msghdr *msg, size_t size, memcpy(ctx->iv, con.iv->iv, ivsize); ctx->aead_assoclen = con.aead_assoclen; - ctx->init = true; } while (size) { diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c index d48d2156e621..749fe42315be 100644 --- a/crypto/algif_aead.c +++ b/crypto/algif_aead.c @@ -106,7 +106,7 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr *msg, size_t usedpages = 0; /* [in] RX bufs to be used from user */ size_t processed = 0; /* [in] TX bufs to be consumed */ - if (!ctx->init || ctx->more) { + if (ctx->more) { err = af_alg_wait_for_data(sk, flags, 0); if (err) return err; diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c index a51ba22fef58..5b6fa5e8c00d 100644 --- a/crypto/algif_skcipher.c +++ b/crypto/algif_skcipher.c @@ -61,7 +61,7 @@ static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg, int err = 0; size_t len = 0; - if (!ctx->init || (ctx->more && ctx->used < bs)) { + if (ctx->more && ctx->used < bs) { err = af_alg_wait_for_data(sk, flags, bs); if (err) return err; diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h index ee6412314f8f..08c087cc89d6 100644 --- a/include/crypto/if_alg.h +++ b/include/crypto/if_alg.h @@ -135,7 +135,6 @@ struct af_alg_async_req { * SG? * @enc: Cryptographic operation to be performed when * recvmsg is invoked. - * @init: True if metadata has been sent. * @len: Length of memory allocated for this data structure. */ struct af_alg_ctx { @@ -152,7 +151,6 @@ struct af_alg_ctx { bool more; bool merge; bool enc; - bool init; unsigned int len; };