All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicolai Stange <nstange@suse.de>
To: Herbert Xu <herbert@gondor.apana.org.au>,
	"David S. Miller" <davem@davemloft.net>
Cc: linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Stephan Müller" <smueller@chronox.de>,
	"Torsten Duwe" <duwe@suse.de>, "Nicolai Stange" <nstange@suse.de>
Subject: [PATCH 6/8] crypto: api - make crypto_alg_lookup() consistently check for failed algos
Date: Sun,  3 Oct 2021 20:14:11 +0200	[thread overview]
Message-ID: <20211003181413.12465-7-nstange@suse.de> (raw)
In-Reply-To: <20211003181413.12465-1-nstange@suse.de>

For convenience, input parameters to crypto_alg_lookup() where
!(mask & CRYPTO_ALG_TESTED) holds are supposed to be equivalent to those
having CRYPTO_ALG_TESTED set in both type and mask.

However the crypto_alg_lookup() code does not behave equivalently in
these two cases, in particular not in regard to the additional check for
failed algorithms introduced with commit eb02c38f0197 ("crypto: api - Keep
failed instances alive").

That is, if the user had explicitly set CRYPTO_ALG_TESTED in mask, the
additional check for failed algorithms would never get to run.

Make crypto_alg_lookup() behave the same for parameter sets considered
equivalent.

Note that with the current code, the first invocation of
__crypto_alg_lookup() from crypto_alg_lookup() always receives a mask with
CRYPTO_ALG_TESTED being set: either because that flag had been set from the
beginning or because the local variable 'test' has been set to
CRYPTO_ALG_TESTED otherwise. As lookup larvals always have
CRYPTO_ALG_TESTED included in their ->mask by now, the first
__crypto_alg_lookup() is guaranteed to find such lookup larvals. In
particular, the second call of __crypto_alg_lookup(), which used to get
invoked with a mask having CRYPTO_ALG_TESTED clear, is not needed anymore
for matching any existing lookup larvals and, in fact, won't find any. In
this context, also c.f. commit b346e492d712 ("crypto: api - fix finding
algorithm currently being tested").

Moreover, because testing larvals have CRYPTO_ALG_TESTED set in their
->type, the first __crypto_alg_lookup() is and always has been able to
find those, for any suitable initial combination of mask and value.

In summary,
- the second __crypto_alg_lookup() won't ever return a larval by now and
- invoking it with the original values of mask and value, i.e. those
  specified by the user at function entry, is not needed anymore.

As it currently stands, the only purpose of that second
__crypto_alg_lookup() invocation is to check whether all matching
algorithms, if any, have failed their selftests.

This allows  for the elimination of the local 'test' variable and some
minor code simplification in crypto_alg_lookup(). More specifically,
rather than keeping track of whether or not CRYPTO_ALG_TESTED had
initially been set via said local 'test', simply set CRYPTO_ALG_TESTED
in both mask and type at function entry if it had initially been unset in
mask. That is, enforce equivalent behaviour for parameter sets considered
equivalent.

Adapt the if-condition guarding that second __crypto_alg_lookup()
invocation as well as the invocation itself accordingly: the search is to
be conducted only in the common case of the user asking for tested
algorithms and it should watch out for failed algorithms, i.e. those
crypto_alg instances having CRYPTO_ALG_TESTED clear in their ->type.

Signed-off-by: Nicolai Stange <nstange@suse.de>
---
 crypto/api.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/crypto/api.c b/crypto/api.c
index 71406f486c65..b96b65b3d5c7 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -237,18 +237,24 @@ static struct crypto_alg *crypto_alg_lookup(const char *name, u32 type,
 					    u32 mask)
 {
 	struct crypto_alg *alg;
-	u32 test = 0;
 
 	if (!(mask & CRYPTO_ALG_TESTED)) {
 		WARN_ON_ONCE(type & CRYPTO_ALG_TESTED);
-		test |= CRYPTO_ALG_TESTED;
+		mask |= CRYPTO_ALG_TESTED;
+		type |= CRYPTO_ALG_TESTED;
 	}
 
 	down_read(&crypto_alg_sem);
-	alg = __crypto_alg_lookup(name, type | test, mask | test);
-	if (!alg && test) {
+	alg = __crypto_alg_lookup(name, type, mask);
+	if (!alg && (type & CRYPTO_ALG_TESTED)) {
+		/*
+		 * Check whether there's an instance which failed the
+		 * selftests in order to avoid pointless retries.
+		 */
+		type &= ~CRYPTO_ALG_TESTED;
 		alg = __crypto_alg_lookup(name, type, mask);
-		if (alg && !crypto_is_larval(alg)) {
+		WARN_ON_ONCE(alg && crypto_is_larval(alg));
+		if (alg) {
 			/* Test failed */
 			crypto_mod_put(alg);
 			alg = ERR_PTR(-ELIBBAD);
-- 
2.26.2


  parent reply	other threads:[~2021-10-03 18:15 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-03 18:14 [PATCH 0/8] crypto: api - priorize tested algorithms in lookup Nicolai Stange
2021-10-03 18:14 ` [PATCH 1/8] crypto: af_alg - reject requests for untested algorithms Nicolai Stange
2021-10-03 18:14 ` [PATCH 2/8] crypto: user " Nicolai Stange
2021-10-03 18:14 ` [PATCH 3/8] crypto: api - only support lookups for specific CRYPTO_ALG_TESTED status Nicolai Stange
2021-10-03 18:14 ` [PATCH 4/8] crypto: api - don't add larvals for !(type & CRYPTO_ALG_TESTED) lookups Nicolai Stange
2021-10-03 18:14 ` [PATCH 5/8] crypto: api - always set CRYPTO_ALG_TESTED in lookup larvals' ->mask/type Nicolai Stange
2021-10-03 18:14 ` Nicolai Stange [this message]
2021-10-03 18:14 ` [PATCH 7/8] crypto: api - lift common mask + type adjustment to crypto_larval_lookup() Nicolai Stange
2021-10-03 18:14 ` [PATCH 8/8] crypto: api - make the algorithm lookup priorize non-larvals Nicolai Stange
2021-10-08 11:54   ` Herbert Xu
2021-10-11  8:34     ` Nicolai Stange
2021-10-22 11:51       ` Herbert Xu
2021-10-27  9:59         ` Nicolai Stange
2021-10-28  2:42           ` Herbert Xu

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=20211003181413.12465-7-nstange@suse.de \
    --to=nstange@suse.de \
    --cc=davem@davemloft.net \
    --cc=duwe@suse.de \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=smueller@chronox.de \
    /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.