All of lore.kernel.org
 help / color / mirror / Atom feed
From: Herbert Xu <herbert@gondor.apana.org.au>
To: Naresh Kamboju <naresh.kamboju@linaro.org>
Cc: Eric Biggers <ebiggers@kernel.org>,
	Luis Chamberlain <mcgrof@kernel.org>,
	LTP List <ltp@lists.linux.it>,
	open list <linux-kernel@vger.kernel.org>,
	linux-security-module@vger.kernel.org, keyrings@vger.kernel.org,
	lkft-triage@lists.linaro.org,
	Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
	Jan Stancek <jstancek@redhat.com>, chrubis <chrubis@suse.cz>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	James Morris <jmorris@namei.org>,
	Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	David Howells <dhowells@redhat.com>,
	"David S. Miller" <davem@davemloft.net>,
	Sachin Sant <sachinp@linux.vnet.ibm.com>,
	Linux Next Mailing List <linux-next@vger.kernel.org>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	linux- stable <stable@vger.kernel.org>
Subject: [v2 PATCH] crypto: af_alg - Fix regression on empty requests
Date: Thu, 2 Jul 2020 13:32:21 +1000	[thread overview]
Message-ID: <20200702033221.GA19367@gondor.apana.org.au> (raw)
In-Reply-To: <CA+G9fYutuU55iL_6Qrk3oG3iq-37PaxvtA4KnEQHuLH9YpH-QA@mail.gmail.com>

On Tue, Jun 30, 2020 at 02:18:11PM +0530, Naresh Kamboju wrote:
> 
> 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

Actually this test really is broken.  Even though empty requests
are legal, they should never be done with no write(2) at all.
Because this fundamentally breaks the use of a blocking read(2)
to wait for more data.

Granted this has been broken since 2017 but I'm not going to
reintroduce this just because of a broken test case.

So please either remove af_alg02 or fix it by adding a control
message through sendmsg(2).

Thanks,

---8<---
Some user-space programs rely on crypto requests that have no
control metadata.  This broke when a check was added to require
the presence of control metadata with the ctx->init flag.

This patch fixes the regression by setting ctx->init as long as
one sendmsg(2) has been made, with or without a control message.

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>

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 9fcb91ea10c41..5882ed46f1adb 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -851,6 +851,7 @@ int af_alg_sendmsg(struct socket *sock, struct msghdr *msg, size_t size,
 		err = -EINVAL;
 		goto unlock;
 	}
+	ctx->init = true;
 
 	if (init) {
 		ctx->enc = enc;
@@ -858,7 +859,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) {
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

WARNING: multiple messages have this Message-ID (diff)
From: Herbert Xu <herbert@gondor.apana.org.au>
To: Naresh Kamboju <naresh.kamboju@linaro.org>
Cc: Eric Biggers <ebiggers@kernel.org>,
	Luis Chamberlain <mcgrof@kernel.org>,
	LTP List <ltp@lists.linux.it>,
	open list <linux-kernel@vger.kernel.org>,
	linux-security-module@vger.kernel.org, keyrings@vger.kernel.org,
	lkft-triage@lists.linaro.org,
	Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
	Jan Stancek <jstancek@redhat.com>, chrubis <chrubis@suse.cz>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	James Morris <jmorris@namei.org>,
	Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	David Howells <dhowells@redhat.com>,
	"David S. Miller" <davem@davemloft.net>,
	Sachin Sant <sachinp@linux.vnet.ibm.com>,
	Linux Next Mailing List <linux-next@vger.kernel.org>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	linux- stable <stable@vger.kernel.org>
Subject: [v2 PATCH] crypto: af_alg - Fix regression on empty requests
Date: Thu, 02 Jul 2020 03:32:21 +0000	[thread overview]
Message-ID: <20200702033221.GA19367@gondor.apana.org.au> (raw)
In-Reply-To: <CA+G9fYutuU55iL_6Qrk3oG3iq-37PaxvtA4KnEQHuLH9YpH-QA@mail.gmail.com>

On Tue, Jun 30, 2020 at 02:18:11PM +0530, Naresh Kamboju wrote:
> 
> 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

Actually this test really is broken.  Even though empty requests
are legal, they should never be done with no write(2) at all.
Because this fundamentally breaks the use of a blocking read(2)
to wait for more data.

Granted this has been broken since 2017 but I'm not going to
reintroduce this just because of a broken test case.

So please either remove af_alg02 or fix it by adding a control
message through sendmsg(2).

Thanks,

---8<---
Some user-space programs rely on crypto requests that have no
control metadata.  This broke when a check was added to require
the presence of control metadata with the ctx->init flag.

This patch fixes the regression by setting ctx->init as long as
one sendmsg(2) has been made, with or without a control message.

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>

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 9fcb91ea10c41..5882ed46f1adb 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -851,6 +851,7 @@ int af_alg_sendmsg(struct socket *sock, struct msghdr *msg, size_t size,
 		err = -EINVAL;
 		goto unlock;
 	}
+	ctx->init = true;
 
 	if (init) {
 		ctx->enc = enc;
@@ -858,7 +859,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) {
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

WARNING: multiple messages have this Message-ID (diff)
From: Herbert Xu <herbert@gondor.apana.org.au>
To: Naresh Kamboju <naresh.kamboju@linaro.org>
Cc: Sachin Sant <sachinp@linux.vnet.ibm.com>,
	David Howells <dhowells@redhat.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	Luis Chamberlain <mcgrof@kernel.org>,
	lkft-triage@lists.linaro.org,
	open list <linux-kernel@vger.kernel.org>,
	Eric Biggers <ebiggers@kernel.org>,
	Linux Next Mailing List <linux-next@vger.kernel.org>,
	linux-security-module@vger.kernel.org, keyrings@vger.kernel.org,
	Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
	chrubis <chrubis@suse.cz>, linux- stable <stable@vger.kernel.org>,
	James Morris <jmorris@namei.org>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	Jan Stancek <jstancek@redhat.com>, LTP List <ltp@lists.linux.it>,
	"Serge E. Hallyn" <serge@hallyn.com>
Subject: [v2 PATCH] crypto: af_alg - Fix regression on empty requests
Date: Thu, 2 Jul 2020 13:32:21 +1000	[thread overview]
Message-ID: <20200702033221.GA19367@gondor.apana.org.au> (raw)
In-Reply-To: <CA+G9fYutuU55iL_6Qrk3oG3iq-37PaxvtA4KnEQHuLH9YpH-QA@mail.gmail.com>

On Tue, Jun 30, 2020 at 02:18:11PM +0530, Naresh Kamboju wrote:
> 
> 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

Actually this test really is broken.  Even though empty requests
are legal, they should never be done with no write(2) at all.
Because this fundamentally breaks the use of a blocking read(2)
to wait for more data.

Granted this has been broken since 2017 but I'm not going to
reintroduce this just because of a broken test case.

So please either remove af_alg02 or fix it by adding a control
message through sendmsg(2).

Thanks,

---8<---
Some user-space programs rely on crypto requests that have no
control metadata.  This broke when a check was added to require
the presence of control metadata with the ctx->init flag.

This patch fixes the regression by setting ctx->init as long as
one sendmsg(2) has been made, with or without a control message.

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>

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 9fcb91ea10c41..5882ed46f1adb 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -851,6 +851,7 @@ int af_alg_sendmsg(struct socket *sock, struct msghdr *msg, size_t size,
 		err = -EINVAL;
 		goto unlock;
 	}
+	ctx->init = true;
 
 	if (init) {
 		ctx->enc = enc;
@@ -858,7 +859,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) {
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

WARNING: multiple messages have this Message-ID (diff)
From: Herbert Xu <herbert@gondor.apana.org.au>
To: ltp@lists.linux.it
Subject: [LTP] [v2 PATCH] crypto: af_alg - Fix regression on empty requests
Date: Thu, 2 Jul 2020 13:32:21 +1000	[thread overview]
Message-ID: <20200702033221.GA19367@gondor.apana.org.au> (raw)
In-Reply-To: <CA+G9fYutuU55iL_6Qrk3oG3iq-37PaxvtA4KnEQHuLH9YpH-QA@mail.gmail.com>

On Tue, Jun 30, 2020 at 02:18:11PM +0530, Naresh Kamboju wrote:
> 
> 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

Actually this test really is broken.  Even though empty requests
are legal, they should never be done with no write(2) at all.
Because this fundamentally breaks the use of a blocking read(2)
to wait for more data.

Granted this has been broken since 2017 but I'm not going to
reintroduce this just because of a broken test case.

So please either remove af_alg02 or fix it by adding a control
message through sendmsg(2).

Thanks,

---8<---
Some user-space programs rely on crypto requests that have no
control metadata.  This broke when a check was added to require
the presence of control metadata with the ctx->init flag.

This patch fixes the regression by setting ctx->init as long as
one sendmsg(2) has been made, with or without a control message.

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>

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 9fcb91ea10c41..5882ed46f1adb 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -851,6 +851,7 @@ int af_alg_sendmsg(struct socket *sock, struct msghdr *msg, size_t size,
 		err = -EINVAL;
 		goto unlock;
 	}
+	ctx->init = true;
 
 	if (init) {
 		ctx->enc = enc;
@@ -858,7 +859,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) {
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

  reply	other threads:[~2020-07-02  3:33 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-22 18:34 LTP: crypto: af_alg02 regression on linux-next 20200621 tag Naresh Kamboju
2020-06-22 18:46 ` Naresh Kamboju
2020-06-22 18:34 ` [LTP] " Naresh Kamboju
2020-06-22 22:49 ` Luis Chamberlain
2020-06-22 22:49   ` [LTP] " Luis Chamberlain
2020-06-23  6:23   ` Naresh Kamboju
2020-06-23  6:35     ` Naresh Kamboju
2020-06-23  6:23     ` [LTP] " Naresh Kamboju
2020-06-23  6:40     ` Herbert Xu
2020-06-23  6:40       ` [LTP] " Herbert Xu
2020-06-23  6:40       ` Herbert Xu
2020-06-23 17:02       ` Eric Biggers
2020-06-23 17:02         ` [LTP] " Eric Biggers
2020-06-23 17:02         ` Eric Biggers
2020-06-24  0:23         ` Herbert Xu
2020-06-24  0:23           ` [LTP] " Herbert Xu
2020-06-24  0:23           ` Herbert Xu
2020-06-26  6:29         ` [PATCH] crypto: af_alg - Fix regression on empty requests Herbert Xu
2020-06-26  6:29           ` [LTP] " Herbert Xu
2020-06-26  6:29           ` Herbert Xu
2020-06-26  6:29           ` Herbert Xu
2020-06-27  8:31           ` Herbert Xu
2020-06-27  8:31             ` Herbert Xu
2020-06-27  8:31             ` [LTP] " Herbert Xu
2020-06-29  8:53             ` [LKP] " Naresh Kamboju
2020-06-29  8:53               ` Naresh Kamboju
2020-06-29  8:53               ` [LTP] [LKP] " Naresh Kamboju
2020-06-30  8:48           ` Naresh Kamboju
2020-06-30  8:48             ` Naresh Kamboju
2020-06-30  8:48             ` [LTP] " Naresh Kamboju
2020-06-30  8:48             ` Naresh Kamboju
2020-07-02  3:32             ` Herbert Xu [this message]
2020-07-02  3:32               ` [LTP] [v2 PATCH] " Herbert Xu
2020-07-02  3:32               ` Herbert Xu
2020-07-02  3:32               ` Herbert Xu
2020-07-03 13:35               ` Luis Chamberlain
2020-07-03 13:35                 ` [LTP] " Luis Chamberlain
2020-07-03 13:35                 ` Luis Chamberlain
2020-07-03 13:35                 ` Luis Chamberlain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200702033221.GA19367@gondor.apana.org.au \
    --to=herbert@gondor.apana.org.au \
    --cc=chrubis@suse.cz \
    --cc=davem@davemloft.net \
    --cc=dhowells@redhat.com \
    --cc=ebiggers@kernel.org \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=jmorris@namei.org \
    --cc=jstancek@redhat.com \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-next@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=lkft-triage@lists.linaro.org \
    --cc=ltp@lists.linux.it \
    --cc=mcgrof@kernel.org \
    --cc=naresh.kamboju@linaro.org \
    --cc=sachinp@linux.vnet.ibm.com \
    --cc=serge@hallyn.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.