On Fri, Dec 06, 2019 at 02:41:55PM -0800, Eric Biggers wrote: > > This approach seems too error-prone, since the prototype of crypto_grab_aead() > doesn't give any indication that it takes a reference to the algorithm which the > caller *must* drop. Fair point. > How about returning the alg pointer in the last argument, similar to > skcipher_alloc_instance_simple()? I know you sent a patch to remove that > argument, but I think it's better to have it... You probably guessed that I don't really like returning two objects from the same function :) So how about this: we let the Crypto API manage the refcount and hide it from all the users. Something like this patch: diff --git a/crypto/algapi.c b/crypto/algapi.c index adb516380be9..34473ab992f2 100644 --- a/crypto/algapi.c +++ b/crypto/algapi.c @@ -563,6 +563,7 @@ int crypto_register_instance(struct crypto_template *tmpl, struct crypto_instance *inst) { struct crypto_larval *larval; + struct crypto_spawn *spawn; int err; err = crypto_check_alg(&inst->alg); @@ -588,6 +589,9 @@ int crypto_register_instance(struct crypto_template *tmpl, if (IS_ERR(larval)) goto err; + hlist_for_each_entry(spawn, &inst->spawn_list, spawn_list) + crypto_mod_put(spawn->alg); + crypto_wait_for_test(larval); err = 0; @@ -623,6 +627,7 @@ int crypto_init_spawn(struct crypto_spawn *spawn, struct crypto_alg *alg, spawn->inst = inst; spawn->mask = mask; + hlist_add_head(&spawn->spawn_list, &inst->spawn_list); down_write(&crypto_alg_sem); if (!crypto_is_moribund(alg)) { @@ -674,6 +679,9 @@ void crypto_drop_spawn(struct crypto_spawn *spawn) if (!spawn->dead) list_del(&spawn->list); up_write(&crypto_alg_sem); + + if (hlist_unhashed(&spawn->inst->list)) + crypto_mod_put(spawn->alg); } EXPORT_SYMBOL_GPL(crypto_drop_spawn); diff --git a/include/crypto/algapi.h b/include/crypto/algapi.h index 771a295ac755..284e96f2eda2 100644 --- a/include/crypto/algapi.h +++ b/include/crypto/algapi.h @@ -48,6 +48,7 @@ struct crypto_instance { struct crypto_template *tmpl; struct hlist_node list; + struct hlist_head spawn_list; void *__ctx[] CRYPTO_MINALIGN_ATTR; }; @@ -66,6 +67,7 @@ struct crypto_template { struct crypto_spawn { struct list_head list; + struct hlist_node spawn_list; struct crypto_alg *alg; struct crypto_instance *inst; const struct crypto_type *frontend; Thanks, -- Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt