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>

Reply via email to