| Message ID | 20190920043556.GP2879@gauss3.secunet.de (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: pcrypt - forbid recursive instantiation | expand |
On Fri, Sep 20, 2019 at 06:35:56AM +0200, Steffen Klassert wrote: > > Fix this by making pcrypt forbid instantiation if pcrypt appears in the > underlying ->cra_driver_name or if an underlying algorithm needs a > fallback. This is somewhat of a hack, but it's a simple fix that should > be sufficient to prevent the deadlock. This still doesn't resolve the case where pcrypt is used in a non-transparent fashion, e.g., through a fallback. Note that even adding a NEED_FALLBACK flag won't fix this as you can construct an instance on top of a NEED_FALLBACK algorithm that itself does not need a fallback. Anyway I decided to change pcrypt to use a set of queues per instance rather than globally and it should resolve the problem. Cheers,
diff --git a/crypto/pcrypt.c b/crypto/pcrypt.c index 543792e0ebf0..932a77b61b47 100644 --- a/crypto/pcrypt.c +++ b/crypto/pcrypt.c @@ -198,6 +198,12 @@ static void pcrypt_free(struct aead_instance *inst) static int pcrypt_init_instance(struct crypto_instance *inst, struct crypto_alg *alg) { + /* Recursive pcrypt deadlocks due to the shared padata_instance */ + if (!strncmp(alg->cra_driver_name, "pcrypt(", 7) || + strstr(alg->cra_driver_name, "(pcrypt(") || + strstr(alg->cra_driver_name, ",pcrypt(")) + return -EINVAL; + if (snprintf(inst->alg.cra_driver_name, CRYPTO_MAX_ALG_NAME, "pcrypt(%s)", alg->cra_driver_name) >= CRYPTO_MAX_ALG_NAME) return -ENAMETOOLONG; @@ -236,7 +242,7 @@ static int pcrypt_create_aead(struct crypto_template *tmpl, struct rtattr **tb, ctx = aead_instance_ctx(inst); crypto_set_aead_spawn(&ctx->spawn, aead_crypto_instance(inst)); - err = crypto_grab_aead(&ctx->spawn, name, 0, 0); + err = crypto_grab_aead(&ctx->spawn, name, 0, CRYPTO_ALG_NEED_FALLBACK); if (err) goto out_free_inst;
If the pcrypt template is used multiple times in an algorithm, then a deadlock occurs because all pcrypt instances share the same padata_instance, which completes requests in the order submitted. That is, the inner pcrypt request waits for the outer pcrypt request while the outer request is already waiting for the inner. Fix this by making pcrypt forbid instantiation if pcrypt appears in the underlying ->cra_driver_name or if an underlying algorithm needs a fallback. This is somewhat of a hack, but it's a simple fix that should be sufficient to prevent the deadlock. Reproducer: #include <linux/if_alg.h> #include <sys/socket.h> #include <unistd.h> int main() { struct sockaddr_alg addr = { .salg_type = "aead", .salg_name = "pcrypt(pcrypt(rfc4106-gcm-aesni))" }; int algfd, reqfd; char buf[32] = { 0 }; algfd = socket(AF_ALG, SOCK_SEQPACKET, 0); bind(algfd, (void *)&addr, sizeof(addr)); setsockopt(algfd, SOL_ALG, ALG_SET_KEY, buf, 20); reqfd = accept(algfd, 0, 0); write(reqfd, buf, 32); read(reqfd, buf, 16); } Reported-by: syzbot+56c7151cad94eec37c521f0e47d2eee53f9361c4@syzkaller.appspotmail.com Fixes: 5068c7a883d1 ("crypto: pcrypt - Add pcrypt crypto parallelization wrapper") Cc: <stable@vger.kernel.org> # v2.6.34+ Signed-off-by: Eric Biggers <ebiggers@google.com> [SK: also require that the underlying algorithm doesn't need a fallback] Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com> --- crypto/pcrypt.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)