Re: [RFC PATCH] crypto: pcrypt - forbid recursive instantiation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Mar 10, 2018 at 03:22:31PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@xxxxxxxxxx>
> 
> 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+56c7151cad94eec37c521f0e47d2eee53f9361c4@xxxxxxxxxxxxxxxxxxxxxxxxx
> Fixes: 5068c7a883d1 ("crypto: pcrypt - Add pcrypt crypto parallelization wrapper")
> Cc: <stable@xxxxxxxxxxxxxxx> # v2.6.34+
> Signed-off-by: Eric Biggers <ebiggers@xxxxxxxxxx>
> ---
>  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.klassert@xxxxxxxxxxx>



[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]

  Powered by Linux