On Sat, Mar 10, 2018 at 03:22:31PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebigg...@google.com>
>
> 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. 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+56c7151cad94eec37c521f0e47d2eee53f936...@syzkaller.appspotmail.com
> Fixes: 5068c7a883d1 ("crypto: pcrypt - Add pcrypt crypto parallelization
> wrapper")
> Cc: <sta...@vger.kernel.org> # v2.6.34+
> Signed-off-by: Eric Biggers <ebigg...@google.com>
> ---
> crypto/pcrypt.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/crypto/pcrypt.c b/crypto/pcrypt.c
> index f8ec3d4ba4a80..3ec64604f6a56 100644
> --- a/crypto/pcrypt.c
> +++ b/crypto/pcrypt.c
> @@ -265,6 +265,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;
> +
This looks ok to me.
Acked-by: Steffen Klassert <steffen.klass...@secunet.com>
