[PATCH AUTOSEL 4.14 39/39] crypto: xts - simplify error handling in ->create()

[PATCH AUTOSEL 4.14 39/39] crypto: xts - simplify error handling in ->create()

[Sasha Levin]

From: Eric Biggers <ebiggers@google.com>

[ Upstream commit 732e540953477083082e999ff553622c59cffd5f ]

Simplify the error handling in the XTS template's ->create() function by
taking advantage of crypto_drop_skcipher() now accepting (as a no-op) a
spawn that hasn't been grabbed yet.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 crypto/xts.c | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/crypto/xts.c b/crypto/xts.c
index e31828ed00466..c8ab2e0cda47c 100644
--- a/crypto/xts.c
+++ b/crypto/xts.c
@@ -525,15 +525,15 @@ static int create(struct crypto_template *tmpl, struct rtattr **tb)
 
 	err = -EINVAL;
 	if (alg->base.cra_blocksize != XTS_BLOCK_SIZE)
-		goto err_drop_spawn;
+		goto err_free_inst;
 
 	if (crypto_skcipher_alg_ivsize(alg))
-		goto err_drop_spawn;
+		goto err_free_inst;
 
 	err = crypto_inst_setname(skcipher_crypto_instance(inst), "xts",
 				  &alg->base);
 	if (err)
-		goto err_drop_spawn;
+		goto err_free_inst;
 
 	err = -EINVAL;
 	cipher_name = alg->base.cra_name;
@@ -546,20 +546,20 @@ static int create(struct crypto_template *tmpl, struct rtattr **tb)
 
 		len = strlcpy(ctx->name, cipher_name + 4, sizeof(ctx->name));
 		if (len < 2 || len >= sizeof(ctx->name))
-			goto err_drop_spawn;
+			goto err_free_inst;
 
 		if (ctx->name[len - 1] != ')')
-			goto err_drop_spawn;
+			goto err_free_inst;
 
 		ctx->name[len - 1] = 0;
 
 		if (snprintf(inst->alg.base.cra_name, CRYPTO_MAX_ALG_NAME,
 			     "xts(%s)", ctx->name) >= CRYPTO_MAX_ALG_NAME) {
 			err = -ENAMETOOLONG;
-			goto err_drop_spawn;
+			goto err_free_inst;
 		}
 	} else
-		goto err_drop_spawn;
+		goto err_free_inst;
 
 	inst->alg.base.cra_flags = alg->base.cra_flags & CRYPTO_ALG_ASYNC;
 	inst->alg.base.cra_priority = alg->base.cra_priority;
@@ -583,17 +583,11 @@ static int create(struct crypto_template *tmpl, struct rtattr **tb)
 	inst->free = free;
 
 	err = skcipher_register_instance(tmpl, inst);
-	if (err)
-		goto err_drop_spawn;
-
-out:
-	return err;
-
-err_drop_spawn:
-	crypto_drop_skcipher(&ctx->spawn);
+	if (err) {
 err_free_inst:
-	kfree(inst);
-	goto out;
+		free(inst);
+	}
+	return err;
 }
 
 static struct crypto_template crypto_tmpl = {
-- 
2.20.1

Re: [PATCH AUTOSEL 4.14 39/39] crypto: xts - simplify error handling in ->create()

[Eric Biggers]

On Thu, May 14, 2020 at 02:54:56PM -0400, Sasha Levin wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> [ Upstream commit 732e540953477083082e999ff553622c59cffd5f ]
> 
> Simplify the error handling in the XTS template's ->create() function by
> taking advantage of crypto_drop_skcipher() now accepting (as a no-op) a
> spawn that hasn't been grabbed yet.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> Signed-off-by: Sasha Levin <sashal@kernel.org>

Please don't backport this patch.  It's a cleanup (not a fix) that depends on
patches in 5.6, which you don't seem to be backporting.

Note, this comment applies to all stable trees as well as all the other
"simplify error handling in ->create()" patches.
I hope that I don't have to reply to every individual email.

- Eric

Re: [PATCH AUTOSEL 4.14 39/39] crypto: xts - simplify error handling in ->create()

[Sasha Levin]

On Thu, May 14, 2020 at 12:08:43PM -0700, Eric Biggers wrote:
>On Thu, May 14, 2020 at 02:54:56PM -0400, Sasha Levin wrote:
>> From: Eric Biggers <ebiggers@google.com>
>>
>> [ Upstream commit 732e540953477083082e999ff553622c59cffd5f ]
>>
>> Simplify the error handling in the XTS template's ->create() function by
>> taking advantage of crypto_drop_skcipher() now accepting (as a no-op) a
>> spawn that hasn't been grabbed yet.
>>
>> Signed-off-by: Eric Biggers <ebiggers@google.com>
>> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>> Signed-off-by: Sasha Levin <sashal@kernel.org>
>
>Please don't backport this patch.  It's a cleanup (not a fix) that depends on
>patches in 5.6, which you don't seem to be backporting.

For 5.6-4.19 I grabbed these to take:

	1a263ae60b04 ("gcc-10: avoid shadowing standard library 'free()' in crypto")

cleanly. I'll drop it as it's mostly to avoid silly gcc10 warnings, but
I just wanted to let you know the reason they ended up here.

>Note, this comment applies to all stable trees as well as all the other
>"simplify error handling in ->create()" patches.
>I hope that I don't have to reply to every individual email.

You don't :)

-- 
Thanks,
Sasha

Re: [PATCH AUTOSEL 4.14 39/39] crypto: xts - simplify error handling in ->create()

[Eric Biggers]

On Thu, May 14, 2020 at 08:55:30PM -0400, Sasha Levin wrote:
> On Thu, May 14, 2020 at 12:08:43PM -0700, Eric Biggers wrote:
> > On Thu, May 14, 2020 at 02:54:56PM -0400, Sasha Levin wrote:
> > > From: Eric Biggers <ebiggers@google.com>
> > > 
> > > [ Upstream commit 732e540953477083082e999ff553622c59cffd5f ]
> > > 
> > > Simplify the error handling in the XTS template's ->create() function by
> > > taking advantage of crypto_drop_skcipher() now accepting (as a no-op) a
> > > spawn that hasn't been grabbed yet.
> > > 
> > > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> > > Signed-off-by: Sasha Levin <sashal@kernel.org>
> > 
> > Please don't backport this patch.  It's a cleanup (not a fix) that depends on
> > patches in 5.6, which you don't seem to be backporting.
> 
> For 5.6-4.19 I grabbed these to take:
> 
> 	1a263ae60b04 ("gcc-10: avoid shadowing standard library 'free()' in crypto")
> 
> cleanly. I'll drop it as it's mostly to avoid silly gcc10 warnings, but
> I just wanted to let you know the reason they ended up here.
> 

If the gcc 10 warning fix is needed, then you should just backport it on its
own.  It just renames a function, so it seems it's trivial to fix the conflict?

- Eric